Closed Bug 1418494 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: XUL, task)

task
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

(Blocks 1 open bug)

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
https://hg.mozilla.org/mozilla-central/rev/79ac723d679b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336
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.