Closed Bug 943888 Opened 6 years ago Closed 4 years ago

Star button should never be disabled

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 49
Tracking Status
firefox49 --- verified

People

(Reporter: sevaan, Assigned: mbrubeck)

References

Details

Attachments

(1 file)

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.
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.
Flags: needinfo?(sfranks)
(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.
Flags: needinfo?(sfranks)
No longer blocks: australis
Thanks! Marking all/all because this behaviour is the same everywhere. :-)
OS: Mac OS X → All
Hardware: x86 → All
Summary: Star icon is disabled when URL is missing from address bar → Star icon is hidden/disabled when URL is missing from address bar
Whiteboard: [Australis:P?]
Duplicate of this bug: 983838
Summary: Star icon is hidden/disabled when URL is missing from address bar → Star button should never be disabled
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.
QA Contact: mbrubeck
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
QA Contact: mbrubeck
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.
Attachment #8745530 - Flags: review?(mak77)
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/
Attachment #8745530 - Flags: review?(mak77)
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.
Attachment #8745530 - Flags: review?(mak77) → review+
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.
https://hg.mozilla.org/mozilla-central/rev/c3446eb8b5e7
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
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
QA Whiteboard: [bugday-20160629]
Duplicate of this bug: 614197
Status: RESOLVED → VERIFIED
Duplicate of this bug: 1261782
Depends on: 1327505
You need to log in before you can comment on or make changes to this bug.