Sevaan Franks [:sevaan] Reporter
	Description • 11 years ago

To bookmark a page, you simply need to click the Star icon in the toolbar. However, if the URL has been removed from the address bar, the star becomes disabled.

In order to reactivate the Star you need to refresh the page to recover the page URL.

How it should work:

The star button should always bookmark the page/file open in the tab, regardless of whether the URL is missing from the address bar.

	:Gijs (he/him)
	Comment 1 • 11 years ago

I'm not sure this should block Australis. On beta, this works exactly the same: the star button disappears out of the address field when you clear the URL. Sevaan, do you think this is more severe a problem than it was before Australis? If so, we could keep it, but otherwise I'd like to remove this from our tracked bug list.

	Sevaan Franks [:sevaan] Reporter
	Comment 2 • 11 years ago

(In reply to :Gijs Kruitbosch from comment #1)
> I'm not sure this should block Australis. On beta, this works exactly the
> same: the star button disappears out of the address field when you clear the
> URL. Sevaan, do you think this is more severe a problem than it was before
> Australis? If so, we could keep it, but otherwise I'd like to remove this
> from our tracked bug list.


No, let's remove it. Sorry, I tested it in the release version of FF and the results were the same, but I had already begun filling the form out as an Australis bug. Forgot to remove the block.

	:Gijs (he/him)
	Comment 3 • 11 years ago

Thanks! Marking all/all because this behaviour is the same everywhere. :-)

	Marco Bonardo [:mak]
	Comment 5 • 8 years ago

now that the star is detached from the urlbar in its own button, there's no compelling reason to keep around the disabling-star code. it should just always be enabled.

	Matt Brubeck (:mbrubeck) Assignee
	Comment 6 • 8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e80201ed1277

	Matt Brubeck (:mbrubeck) Assignee
	Comment 7 • 8 years ago

Review commit: https://reviewboard.mozilla.org/r/49041/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49041/

	Marco Bonardo [:mak]
	Comment 8 • 8 years ago

Comment on attachment 8745530 [details]
MozReview Request: Bug 943888 - Always keep the bookmark star button enabled [r?mak]

https://reviewboard.mozilla.org/r/49041/#review46271

::: browser/base/content/browser-places.js:1415
(Diff revision 1)
>    },
>  
>    /**
>     * Handles star styling based on page proxy state changes.
>     */
>    onPageProxyStateChanged: function BUI_onPageProxyStateChanged(aState) {

Do we still need to handle onPageProxyStateChanged at all? Isn't onLocationChange enough if we only care about content and not the urlbar contents?

Could be I'm missing something after so much time I don't look into this code, so please let me know if you noticed some misbehaviors.

	Matt Brubeck (:mbrubeck) Assignee
	Comment 9 • 8 years ago

Comment on attachment 8745530 [details]
MozReview Request: Bug 943888 - Always keep the bookmark star button enabled [r?mak]

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49041/diff/1-2/

	Marco Bonardo [:mak]
	Comment 10 • 8 years ago

Comment on attachment 8745530 [details]
MozReview Request: Bug 943888 - Always keep the bookmark star button enabled [r?mak]

https://reviewboard.mozilla.org/r/49041/#review46365

LGTM, thank you.

	Marco Bonardo [:mak]
	Comment 11 • 8 years ago

note, I don't recall if any tests are checking the disabled status... off-hand I don't think, but in case it may be useful to have even just a single platform try run with mochitests.

	Pulsebot
	Comment 12 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3446eb8b5e7

	Carsten Book [:Tomcat]
	Comment 13 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/c3446eb8b5e7

	Khalid Syfullah Zaman [:khalid32]
	Comment 14 • 8 years ago

I have reproduced this bug on Nightly 28.0a1(2013-11-27) on Linux, 64 bit!

The bug's fix is now verified on Latest Aurora 49.0a2!

Build ID: 20160629004019
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
OS: Ubuntu 16.04, Linux 4.4.0-28-generic