1542044 - browser.theme.getCurrent() & onUpdated should take in account `dark_theme` manifest field

Status: NEW

(WebExtensions :: Themes, defect, P5)

Comment 1

[David Durst [:ddurst]]

According to bug 1542026, "before bug 1525762, it would reset to the non-dark default theme" -- but this bug is asserting that getCurrent() should be the built-in dark theme when the OS is in dark mode.

It sounds like this is an enhancement request, whereas 1542026 is a regression (though it would seem an enhancement over the previous functionality, if it used to be the case that in OS dark mode reset got you the non-dark default and now it gets you the dark built-in).

Comment 2

[Mike de Boer [:mikedeboer]]

I don't think that reset() and getCurrent() are meant to be measured using the same scale like that. As I understand it, reset() is working as expected for API consumers (theme authors) when it reverts any changes made by theme API consumers during runtime.
getCurrent() is working as expected for API consumers (theme authors) when it returns information that is useful for themes to react to and adapt styles accordingly. If getCurrent() returned the dark styles of the default theme in dark mode, but that somehow isn't the case anymore, this should be fixed.

Tim, could you take another look at the bug Summary and improve it to reflect the problem more clearly, including the fact that this is a regression?

Comment 3

[Tim Nguyen :ntim]

(In reply to Mike de Boer [:mikedeboer] from comment #2)

I don't think that reset() and getCurrent() are meant to be measured using the same scale like that. As I understand it, reset() is working as expected for API consumers (theme authors) when it reverts any changes made by theme API consumers during runtime.

Yes, but reset() not working properly after bug 1525762, see the screenshot in bug 1542026, it resets to a strange white non-default theme. That one is definitively a regression.

getCurrent() is working as expected for API consumers (theme authors) when it returns information that is useful for themes to react to and adapt styles accordingly. If getCurrent() returned the dark styles of the default theme in dark mode, but that somehow isn't the case anymore, this should be fixed.

Bug 1525762 introduced a dark_theme manifest field defining the theme that is applied when the system dark mode is enabled, that information is not taken in account when returning the currently used theme via getCurrent().

It's a regression as in, getCurrent() would no longer return the correct theme if a theme used the dark_theme manifest field (which is currently only used by the default theme in dark mode, but could be used with any theme on AMO technically).

Comment 4

[Mike de Boer [:mikedeboer]]

Right, thanks for the explanation, Tim. It's a bit unfortunate that this isn't covered by automated tests, but shrug.

Kris, it seems pretty clear at this point that this has been regressed by bug 1525762. Can you find time to pick this up? Or perhaps defer to :aswan, since he reviewed the patches (if he happens to have a friendlier schedule than you at this time)?

Comment 5

[Tim Nguyen :ntim]

Attachment: Bug 1542044 - Make theme.getCurrent() take in account dark_theme manifest field.

Comment 8

[Manuel Reimer]

Actually this is not only about "browser.theme.getCurrent()". Probably the bug title needs updating.

I am using "browser.theme.onUpdated" to get notified about theme color changes in my Add-on to "dynamically patch" my browser action icon to have the updated colors. This is currently the only way to have an icon which always fits nice with the default icons, as Mozilla refuses to provide a more convenient solution to this for years. They better keep a "whitelist" of Add-ons who are allowed to have auto-adapting icons.

If the change between "theme" and "dark_theme" happens, then no "onUpdated" event is sent to Add-ons at all.

For me it seems like the WebExtensions backend never was updated to support "dark_theme" in any way.

But the bug is open for two years, so probably no big chance to see a fix soon...

Comment 9

[Simeon Vincent [:dotproto]]

I found a concrete example of a theme in the wild that exposes both "theme" and "dark_theme": the "auto-firefox" version of https://github.com/serverwentdown/matched/. This theme is available on AMO at https://addons.mozilla.org/en-US/firefox/addon/auto-matched/.

When I have that theme installed and switch my OS (macOS) between light and dark mode, Firefox's chrome will reflect the mode change as expected, but the browser.theme.onUpdated event does not fire and browser.theme.getCurrent() always returns the same theme data – the settings declared in the "theme" key of the theme's manifest.json.

As I see it, ideally browser.theme.getCurrent() would return the set of colors that are currently applied to Firefox's chrome. Failing that, it would be preferable if that function returned an object that contained the "theme" and "dark_theme" objects declared in the manifest as that would at least enable extension developers to construct a stylesheet that included colors declared using light-dark(<light_color>, <dark_color>) or @media (prefers-color-scheme: <scheme>) media queries.