Closed Bug 1001995 Opened 10 years ago Closed 10 years ago

Remove / fix remnants of the forwarddisabled attribute

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 32
Tracking Status
firefox31 --- fixed
firefox32 --- fixed

People

(Reporter: quicksaver, Assigned: dao)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file)

This can become confusing for stylesheets, for example after bug 997131 this won't work as expected:
> /* these are so the identity box doesn't jump around when going back/forward in the tab history */
> /* remove: these aren't needed in FF31 */
> window:not([chromehidden~="toolbar"]) #urlbar-container[forwarddisabled] > #urlbar-wrapper > #urlbar > #notification-popup-box[hidden] + #identity-box > #page-proxy-favicon,
> window:not([chromehidden~="toolbar"]) #urlbar-container[forwarddisabled][switchingtabs] + #urlbar-container > #urlbar > #notification-popup-box[hidden] + #identity-box > #page-proxy-favicon,
> /* end remove */
> window:not([chromehidden~="toolbar"]) #urlbar-wrapper > #forward-button[disabled] + #urlbar > #notification-popup-box[hidden] + #identity-box > #page-proxy-favicon {
> 	-moz-margin-start: 0;
> }

That's because, even though the forwarddisabled attribute is no longer set or actually used, #urlbar-container is initialized with it:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul#642

Also, a couple of leftovers that will be affected when removing that attribute:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/browser.css#3382
http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser.css#2277

Dao, once again I'm CC'ing you directly because you were assigned bug 997131.
Sorry, I should have been more specific. What I meant was we can't differentiate between both "versions" of this style through the stylesheet alone, because the forwarddisabled attribute still exists after the patch.
Good find.
Assignee: nobody → dao
Blocks: 1001471
Status: NEW → ASSIGNED
Summary: Since bug 997131 the forwarddisabled attribute can't be relied on → Remove / fix remnants of the forwarddisabled attribute
Flags: firefox-backlog?
Flags: firefox-backlog? → firefox-backlog+
Attached patch patchSplinter Review
Attachment #8414465 - Flags: review?(mdeboer)
Comment on attachment 8414465 [details] [diff] [review]
patch

Review of attachment 8414465 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM! Looks like you got all the [forwarddisabled] occurrences here.
Attachment #8414465 - Flags: review?(mdeboer) → review+
https://hg.mozilla.org/mozilla-central/rev/e2cc89dc7e71
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Comment on attachment 8414465 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 997131
User impact if declined: primarily bug 1001471
Testing completed (on m-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): very innocent changes, lowest risk
String or IDL/UUID changes made by this patch: none
Attachment #8414465 - Flags: approval-mozilla-aurora?
Attachment #8414465 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.