1628701 - Port bug 1624482: Scope menulist styles to its own component using shadow DOM

Status: RESOLVED FIXED

(Thunderbird :: Theme, task)

Description

[Richard Marti (:Paenglab)]

bug 1624482 moved some elements into shadow-DOM.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1628701-scope-menulist.patch

This patch hopefully fixes the CSS. But in mailWidgets.js it needs more work as the editable menulists are broken.

Not really tested as Windows build is busted.

Comment 2

[Richard Marti (:Paenglab)]

Attachment: 1628701-scope-menulist.patch

I checked the normal menulists and they should be okay now.

I played a bit with mailWidgets.js and tried to convert the textbox in the editable menulists the menulist-input and textbox-input classes are now the ::part(text-input) in the CSS. The editable menulist is now almost normally shown but without label and the textbox doesn't work. This needs now a CE expert, like you Magnus, to fix it. ;-)

Comment 3

[Magnus Melin [:mkmelin]]

Attachment: 1628701-editable-part-fix.patch

Some fixes to your patch.
But the hightlightable-label is giving me headaches. It won't stop showing
https://hg.mozilla.org/mozilla-central/rev/86530e261ad5#l19.58

The menulist::part(label-box) rule from messengercompose.css is getting picked up, but not stuff below it. Is it not allowed to select thing below a part, or not working?

Comment 4

[Tim Nguyen :ntim]

Comment on attachment 9139568 [details] [diff] [review]
1628701-editable-part-fix.patch

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

::: mail/base/content/mailWidgets.js
@@ +796,5 @@
>          }
>          this.menupopup.addEventListener("popuphiding", this._popupHiding);
>        }
>  
> +      get fragment() {

You can remove this bit and use `this.constructor.fragment` instead, see bug 1590573

Comment 5

[Tim Nguyen :ntim]

(In reply to Magnus Melin [:mkmelin] from comment #3)

Created attachment 9139568 [details] [diff] [review]
1628701-editable-part-fix.patch

Some fixes to your patch.
But the hightlightable-label is giving me headaches. It won't stop showing
https://hg.mozilla.org/mozilla-central/rev/86530e261ad5#l19.58

The menulist::part(label-box) rule from messengercompose.css is getting picked up, but not stuff below it. Is it not allowed to select thing below a part, or not working?

You should just put <link rel="stylesheet" href="chrome://global/skin/menulist.css"/> inside the markup, and then the rules from https://hg.mozilla.org/mozilla-central/rev/86530e261ad5#l19.58 will apply.

I think that's desired anyway, since menulist-editable used class names from menulist, so you'll want all the styles formerly applying to menulist to apply to menulist-editable children as well.

Alternatively, elements can have multiple parts, so you can use part="label highlightable-label", and since the [highlightable] attribute inherits from the parent, target it as: menulist[highlightable]::part(highlightable-label) and such.

Comment 6

[Magnus Melin [:mkmelin]]

Thanks, that seems to work! I'll fold these and land it.
For toolkit: was the highlightable-label part="label" intended?

There seem to be some minor things still to do (e.g. the description in the menulist doesn't have the same faded color as before), but we can do a followup patch.

Comment 7

[Tim Nguyen :ntim]

Attachment: menulist-tweaks.diff

This needs both Magnus and Richard's patches applied.

• I've imported menulist.css from toolkit into the markup
• I've removed unnecessary IDs and classes
• I've fixed .menulist-description styling
• I've removed some code I think is dead (menulist-editable-box/menulist-editable-input)

Comment 8

[Tim Nguyen :ntim]

(In reply to Magnus Melin [:mkmelin] from comment #6)

Thanks, that seems to work! I'll fold these and land it.
For toolkit: was the highlightable-label part="label" intended?

Yep, part is like class, multiple elements can share the same part. When we style the label, we usually want to style both labels at the same time.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/27f756d7fac7
Port bug 1624482: Scope menulist styles to its own component using shadow DOM. r=mkmelin

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/468302900d21
follow-up - adjust calendar tests for menulist DOM change. rs=bustage-fix

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f6aaa12818d0
follow-up - make browser_menulist aware of the shadow DOM. rs=bustage-fix

Comment 12

[Intermittent Failures Robot]

8 failures in 4823 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 8

Platform breakdown:

• linux32: 2
• windows10-64: 2
• windows7-32: 2
• linux64: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1628701&startday=2020-04-06&endday=2020-04-12&tree=all