Closed Bug 1418494 Opened 7 years ago Closed 7 years ago

Flatten "menucaption-inmenulist" into the "menucaption" binding

Categories

(Core :: XUL, task)

task
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

(Whiteboard: [xbl-flatten-inheritance])

Attachments

(1 file)

AFAICT the only time menucaptions are created are for select optgroups [0], and they always get instantiated as "menucaption-inmenulist" (because it matches the `menulist > menupopup > menucaption` selector [1]). This means the plain "menucaption" selector [2] (which attaches the menucaption XBL binding) never gets used. A search for just "menucaption" is pretty scannable, and I don't see any other elements being created [3]. [0]: https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/SelectParentHelper.jsm#346 [1]: https://dxr.mozilla.org/mozilla-central/source/toolkit/content/xul.css#959 [2]: https://dxr.mozilla.org/mozilla-central/source/toolkit/content/xul.css#390 [3]: https://dxr.mozilla.org/mozilla-central/search?q=menucaption&redirect=true
One way to check for this feature: ./mach run "data:text/html,<select multiple><optgroup label='Group 1'><option>Option 1.1</option></optgroup><optgroup label='Group 2'><option>Option 2.1</option><option>Option 2.2</option></optgroup><optgroup label='Group 3' disabled><option>Option 3.1</option><option>Option 3.2</option><option>Option 3.3</option></optgroup></select>"
Mike, are you aware of a case where we have a <menucaption> that doesn't match the `menulist > menupopup > menucaption` selector?
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Mozscreenshots run: https://screenshots.mattn.ca/compare/?oldProject=try&oldRev=43677ce14c19fc91c8410b7d6341c896ecb7cdb2&newProject=try&newRev=dd7de9aa0bc56180a7937d9ce01f92a79e28ffe6. Most of those changes look like intermittent issues I've seen before. The "devtools_7_styleeditor" change makes sense - the number of rules in xul.css changed so it's picking that up
Comment on attachment 8929628 [details] Bug 1418494 - Flatten menucaption-inmenulist into menucaption; https://reviewboard.mozilla.org/r/200910/#review206366 Great! Thanks for getting rid of that. I think I've found some leftover CSS rules you can tear out to accompany this. ::: toolkit/content/widgets/menu.xml (Diff revision 1) > </content> > </binding> > > - <binding id="menucaption" extends="chrome://global/content/bindings/menu.xml#menu-base"> > - <content> > - <xul:label class="menu-text" xbl:inherits="value=label,crop" crop="right"/> Pretty sure this means that ```css menucaption > .menu-text { /* ... */ } ``` rules can go away. I've found a few: https://searchfox.org/mozilla-central/rev/c633ffa4c4611f202ca11270dcddb7b29edddff8/toolkit/themes/osx/global/menu.css#42 https://searchfox.org/mozilla-central/rev/c633ffa4c4611f202ca11270dcddb7b29edddff8/toolkit/themes/linux/global/menu.css#107 https://searchfox.org/mozilla-central/rev/c633ffa4c4611f202ca11270dcddb7b29edddff8/toolkit/themes/windows/global/menu.css#72
Attachment #8929628 - Flags: review?(mconley) → review+
(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews and needinfos from comment #5) > Pretty sure this means that > > ```css > menucaption > .menu-text { > /* ... */ > } > ``` > > rules can go away. Good catch, I will remove those and double check for anything else
(In reply to Brian Grinstead [:bgrins] from comment #3) > Mike, are you aware of a case where we have a <menucaption> that doesn't > match the `menulist > menupopup > menucaption` selector? Sorry, missed this question. No, I'm not aware of any such case.
Pushed by bgrinstead@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/79ac723d679b Flatten menucaption-inmenulist into menucaption;r=mconley
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Component: XP Toolkit/Widgets: XUL → XUL
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: