1459066 - Overflow menu WebExtension subview header text is invisible on dark theme

Status: RESOLVED FIXED

(WebExtensions :: Frontend, defect)

Description

[Tim Nguyen :ntim]

Attachment: Screenshot

The subview header containing the "<" arrow + the extension title is invisible on dark themes.

I think this is a regression of bug 1449933.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1459066 - Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling.

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

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8973740 [details]
Bug 1459066 - Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling.

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

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8973740 [details]
Bug 1459066 - Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling.

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

Comment 4

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8973740 [details]
Bug 1459066 - Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling.

https://reviewboard.mozilla.org/r/242120/#review248036

::: browser/components/extensions/ExtensionPopups.jsm:363
(Diff revision 3)
> +    if (this.panel.id == "widget-overflow") {
> +      this.browser.style.background = background;
> +    } else {
> -    this.panel.style.setProperty("--arrowpanel-background", background);
> +      this.panel.style.setProperty("--arrowpanel-background", background);
> +    }

Is there any reason we shouldn't just always set the style on browser?

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8973740 [details]
Bug 1459066 - Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242120/diff/3-4/

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8973740 [details]
Bug 1459066 - Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242120/diff/4-5/

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8973740 [details]
Bug 1459066 - Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling.

https://reviewboard.mozilla.org/r/242120/#review248036

> Is there any reason we shouldn't just always set the style on browser?

Because standalone popups should have the styles extend to the popup arrow. If we set the style on the browser all the time, we would still need to apply it to the arrow when it is standalone otherwise in most cases users would see a slight variation in the color of the arrow and top part of the popup box (rgb(252, 252, 252, 0.976) on OSX for example) and the browser (rgb(255, 255, 255)).

Comment 8

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8973740 [details]
Bug 1459066 - Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling.

https://reviewboard.mozilla.org/r/242120/#review248678

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f3cab27183f
Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling. r=mixedpuppy

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/1f3cab27183f

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8973740 [details]
Bug 1459066 - Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1449933
[User impact if declined]: when extensions that provide their own popup are placed in the overflow menu, the header for the popup can be unreadable on dark themes
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: limited to a special case of extension popups
[String changes made/needed]: none

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8973740 [details]
Bug 1459066 - Overflow menu WebExtension subview header should use the default browser styling instead of the extension's styling.

Avoid unreadable text from extension popups with the dark theme enabled under some circumstances. Approved for 61.0b5.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d8660eabf305