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

Status: RESOLVED FIXED

(Core :: XUL, task)

Description

[Brian Grinstead [:bgrins]]

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

Comment 1

[Brian Grinstead [:bgrins]]

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>"

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: Bug 1418494 - Flatten menucaption-inmenulist into menucaption;

The only time menucaptions are created are for optgroups, and they
always get instantiated as menucaption-inmenulist. Flatten the
inheritance here to get rid of a binding.

Review commit: https://reviewboard.mozilla.org/r/200910/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/200910/

Comment 3

[Brian Grinstead [:bgrins]]

Mike, are you aware of a case where we have a <menucaption> that doesn't match the `menulist > menupopup > menucaption` selector?

Comment 4

[Brian Grinstead [:bgrins]]

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 5

[Mike Conley (:mconley) (:⚙️)]

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

Comment 6

[Brian Grinstead [:bgrins]]

(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

Comment 7

[Brian Grinstead [:bgrins]]

Comment on attachment 8929628 [details]
Bug 1418494 - Flatten menucaption-inmenulist into menucaption;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200910/diff/1-2/

Comment 8

[Mike Conley (:mconley) (:⚙️)]

(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.

Comment 9

[Brian Grinstead [:bgrins]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f35e63c611081fe9fbc1b6c3b6218c36b800a3d

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/79ac723d679b
Flatten menucaption-inmenulist into menucaption;r=mconley

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/79ac723d679b

Comment 12

[Firefox Bug Husbandry Bot]

Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336