Closed Bug 1422386 Opened 3 years ago Closed 2 years ago
Remove the 'toolbardecoration' binding
59 bytes, text/x-review-board-request
Just like what we did for menuseparator: we have three tags tied to toolbardecoration: toolbarseparator, toolbarspacer, toolbarspring. - https://dxr.mozilla.org/mozilla-central/search?q=toolbardecoratio&redirect=false Example of what a patch might look like (although the place where the CSS file gets referenced may change based on the outcome of Bug 1420229) https://hg.mozilla.org/try/rev/8b6f96efe45b915f28bc6c89c29b60bad598abc0
The accessibility portion of this will be handled in Bug 1428930. There'll still need to be work done here to move the CSS into components.css and remove toolbardecoration/toolbar-base bindings similar to what the changeset linked in the description does.
Depends on: 1428930
Summary: Remove the 'toolbardecoration' binding and move accessibility into XUL based on tag name instead of XBL role → Remove the 'toolbardecoration' binding
This is unblocked by bug 1428930.
Assignee: bgrinstead → timdream
Comment on attachment 8950569 [details] Bug 1422386 - Remove toolbardecoration, toolbar, and toolbar-base binding, https://reviewboard.mozilla.org/r/219846/#review225632 Looks good to me, but the commit message is outdated, please update it to reflect which bindings could be removed and why.
Attachment #8950569 - Flags: review?(paolo.mozmail) → review+
It's probably worth doing a mozscreenshots run since this is changing the CSS to a UA style. The way I do it usually is: 1. Remove the patch and do `./mach try fuzzy -q "browser-screenshots"` 2. Apply the patch and do `./mach try fuzzy -q "browser-screenshots"` Then plug them into this URL (replacing 1 and 2 with the actual revisions): https://screenshots.mattn.ca/compare/?oldProject=try&oldRev=1&newProject=try&newRev=2
(In reply to Brian Grinstead [:bgrins] from comment #7) > It's probably worth doing a mozscreenshots run since this is changing the > CSS to a UA style. https://screenshots.mattn.ca/compare/?oldProject=try&oldRev=ffda3ab9975f01ea4091f769753f96735c75d62f&newProject=try&newRev=3d916350228ddddb1c4cb56c52adabcedd133804
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #10) > (In reply to Brian Grinstead [:bgrins] from comment #7) > > It's probably worth doing a mozscreenshots run since this is changing the > > CSS to a UA style. > > https://screenshots.mattn.ca/compare/ > ?oldProject=try&oldRev=ffda3ab9975f01ea4091f769753f96735c75d62f&newProject=tr > y&newRev=3d916350228ddddb1c4cb56c52adabcedd133804 Awesome, looks like loading it as a UA style didn't cause any changes (at least on osx/linux as win builds aren't working right now)
OK. We cannot land this yet because there is still one test failure that I haven't figured out why: browser/components/customizableui/test/browser_876926_customize_mode_wrapping.js So far I can "fix" it locally with diff --git a/toolkit/content/widgets/toolbar.xml b/toolkit/content/widgets/toolbar.xml --- a/toolkit/content/widgets/toolbar.xml +++ b/toolkit/content/widgets/toolbar.xml @@ -170,5 +170,5 @@ </binding> - <binding id="toolbarpaletteitem" display="xul:button"> + <binding id="toolbarpaletteitem" extends="chrome://browser/content/tabbrowser.xml#empty" display="xul:button"> <content> <xul:hbox class="toolbarpaletteitem-box" flex="1" xbl:inherits="type,place"> I am in the process of figuring out the root cause.
You need to use extends="xul:button"
(In reply to Neil Deakin from comment #13) > You need to use extends="xul:button" Ah, thanks for the pointer. https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XBL/XBL_1.0_Reference/Elements#binding "The 'display' attribute does not work unless the extends attribute is set, see bug 119389. In that case, put the value of the display attribute into the extends attribute."
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/64c976234100 Remove toolbardecoration, toolbar, and toolbar-base binding, r=Paolo
Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336
Component: XP Toolkit/Widgets: XUL → XUL
You need to log in before you can comment on or make changes to this bug.