Bugzilla

(In reply to Rob Wu [:robwu] from comment #2)
> Thanks for the patch. I'll discuss this with my team next week, but meanwhile I have some questions:
> 
> - Themes/extensions can declare `theme` and `dark_theme`, and the choice is based on some heuristics at https://searchfox.org/mozilla-central/rev/d4b9c457db637fde655592d9e2048939b7ab2854/toolkit/modules/LightweightThemeConsumer.jsm#237-241. Would it make sense to default to the "dark" value for `color_scheme`/`content_color_scheme` for color properties in the `dark_theme` theme?

That seems reasonable, will update the patch and add a test.

> - The [`theme.update` API](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/theme/update) supports window-specific themes. What is the desired (and actual) behavior when `color_scheme` / `content_color_scheme` is used? Could you add some unit tests that check the behavior when `browser.theme.update` is used with window-specific themes?
>   - If window-specific themes cannot be supported for technical reasons, could you describe the current behavior, so we can evaluate whether it makes sense?
>   - Even if window-specific themes can be supported, I wonder whether edge cases such as tabs moving between windows are accounted for (the content color scheme would then have to update based on the window's theme).

In terms of "desired" behavior, in an ideal world that would be supported just fine, but in practice we can't for various reasons:

 * The main one is that GTK doesn't allow us to switch native appearance in a performant way, so in practice all windows need to share the same native theme. Note that the theme still works of course, this mostly affects stuff like context menus / XUL trees / dialogs / etc.

 * Even in increase in complexity in the platform would be significant (as you mentioned we'd need to track the window the current browsing-context is in and so on, and react to changes to it).

So I believe the current behavior is that both browser and content color scheme depend on the last theme.update call, in practice. This behavior is not changed by this patch at all, and this new API shouldn't prevent us from supporting the desired behavior in the future if we decide it's worth the complexity.

(In reply to Rob Wu [:robwu] from comment #2)
> Thanks for the patch. I'll discuss this with my team next week, but meanwhile I have some questions:
> 
> - Themes/extensions can declare `theme` and `dark_theme`, and the choice is based on some heuristics at https://searchfox.org/mozilla-central/rev/d4b9c457db637fde655592d9e2048939b7ab2854/toolkit/modules/LightweightThemeConsumer.jsm#237-241. Would it make sense to default to the "dark" value for `color_scheme`/`content_color_scheme` for color properties in the `dark_theme` theme?

That seems reasonable, will update the patch and add a test.

> - The [`theme.update` API](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/theme/update) supports window-specific themes. What is the desired (and actual) behavior when `color_scheme` / `content_color_scheme` is used? Could you add some unit tests that check the behavior when `browser.theme.update` is used with window-specific themes?
>   - If window-specific themes cannot be supported for technical reasons, could you describe the current behavior, so we can evaluate whether it makes sense?
>   - Even if window-specific themes can be supported, I wonder whether edge cases such as tabs moving between windows are accounted for (the content color scheme would then have to update based on the window's theme).

In terms of "desired" behavior, in an ideal world that would be supported just fine, but in practice we can't for various reasons:

 * The main one is that GTK doesn't allow us to switch native appearance in a performant way, so in practice all windows need to share the same native theme. Note that the theme still works of course, this mostly affects stuff like context menus / XUL trees / dialogs / etc.

 * The increase in complexity in the platform would be significant (as you mentioned we'd need to track the window the current browsing-context is in and so on, and react to changes to it).

So I believe the current behavior is that both browser and content color scheme depend on the last theme.update call, in practice. This behavior is not changed by this patch at all, and this new API shouldn't prevent us from supporting the desired behavior in the future if we decide it's worth the complexity.