1342712 - Allow browser themes to be scoped to a specific window or active tab (like Vivaldi / VivaldiFox)

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect, P5)

Description

[Dan Callahan [:callahad]]

Attachment: Screenshot of Vivaldi

The Vivaldi browser (see screenshot) is able to modify its theme on a per-window basis -- each window's theme is adjusted to match its own frontmost tab.

This is also mimicked by :ntim's VivaldiFox add-on (https://addons.mozilla.org/en-US/firefox/addon/vivaldifox/)

As far as I can tell, the current WebExtension theme proposals are all global -- would it be possible to also have per-window or /per-tab overrides of theme properties to support this use case?

Comment 1

[Dan Callahan [:callahad]]

Attachment: Screenshot of Firefox with VivaldiFox add-on

Comment 2

[Matthew Wein [:mattw]]

You could use the Theme API's `update` function paired with Tabs API to update the theme each time a new tab is activated. I'm not sure if this would work for having multiple windows display different themes, though.

Comment 3

[:shell escalante]

dan, does matts' reply answer your question?

Comment 4

[Dan Callahan [:callahad]]

Matt's solution works for tabs within a single window, but would not make it possible to replicate the effect on multiple windows like in the screenshot.

Comment 5

[:shell escalante]

potentially requiring front end support if doesn't exist.  talk about in theming.

Comment 7

[Dan Callahan [:callahad]]

I don't believe this is a dupe of #1320585.

As far as I can tell, #1320585 is about persistently styling each tab element, independently of whatever other theme-related modifications are in effect. This bug is about applying themes on a per-window basis, ideally tied to the active tab in each window.

Reopening and adjusting the title in an attempt to be more clear.

Comment 8

[Caitlin Neiman [:caitmuenster]]

Hi Dan, this has been added to the agenda for the June 6 WebExtension APIs triage meeting. Would you be able to join us? 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Archive

Agenda: https://docs.google.com/document/d/1pTNjK5i_8gHt3EeiUiu5KCUVeRcfwn7ybCPDBSx6CLM/edit#

Comment 9

[Tim Nguyen :ntim]

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/windows/onFocusChanged + theme.update() should work, but I have no idea if onFocusChanged fires when another app (different than Firefox) gets focused.

Comment 10

[Tim Nguyen :ntim]

Actually, I've realized that this wouldn't work because since the theme is global to Firefox, all windows would have the same inactive/active style no matter what state the window is in. This solution would work for the case where 1 window is opened though

Comment 11

[Dan Callahan [:callahad]]

Hi Caitlin,

I won't be able to attend the triage meeting - I'll be on a flight at that time. If there are any questions I can answer ahead of the meeting, please let me know.

Aside from just mimic Vivaldi's look and feel, which I find quite pleasant, I'd love to be able to create a WebExtension that allowed me to create Container-specific windows. Being able to style each window to match its associated Container (Work, Personal, Shopping, etc.) would greatly help with distinguishing the windows from each other.

Comment 12

[Tim Nguyen :ntim]

How about an optional parameter to browser.theme.update() called tabId ?

Comment 13

[Tim Nguyen :ntim]

or windowId, which is the real blocker for my add-on.

Comment 14

[Tim Nguyen :ntim]

Attachment: Bug 1342712 - Allow scoping a theme per-window.

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

Comment 15

[Tim Nguyen :ntim]

Comment on attachment 8889170 [details]
Bug 1342712 - Allow scoping a theme per-window.

This patch doesn't work yet. I think it's because it's missing some changes in LightweightThemeManager.jsm though I'm not sure what exactly. Do you think you can help me? Thanks!

Comment 16

[Mike de Boer [:mikedeboer]]

Comment on attachment 8889170 [details]
Bug 1342712 - Allow scoping a theme per-window.

I'm on vacation for the coming two weeks - I hope Jared may have time to provide feedback on this patch!

Comment 17

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

Comment on attachment 8889170 [details]
Bug 1342712 - Allow scoping a theme per-window.

https://reviewboard.mozilla.org/r/160268/#review170404

::: toolkit/components/extensions/ext-theme.js:280
(Diff revision 1)
>    getAPI(context) {
>      let {extension} = context;
>  
>      return {
>        theme: {
> -        update: (details) => {
> +        update: (windowId, details) => {

Please change this to:
update: (details, windowId) => {}
since all of the other places where 'details' is passed, 'details' is the first argument.

::: toolkit/modules/LightweightThemeConsumer.jsm:7
(Diff revision 1)
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  this.EXPORTED_SYMBOLS = ["LightweightThemeConsumer"];
>  
>  const {utils: Cu} = Components;

> const {utils: Cu, interfaces: Ci, classes: Cc} = Components;

::: toolkit/modules/LightweightThemeConsumer.jsm:64
(Diff revision 1)
> +      .QueryInterface(Ci.nsIInterfaceRequestor)
> +      .getInterface(Ci.nsIDOMWindowUtils);

Ci is not defined in this file which is why this line was failing.

::: toolkit/modules/LightweightThemeConsumer.jsm:67
(Diff revision 1)
> +    console.log(aData.window, outerWindowID);
> +    if (aData.window && aData.window !== outerWindowID) {

`console` doesn't exist in JSM. If you want to log something while developing, use `Services.console.logStringMessage()`.

aData is still a string at this point, and therefore doing `aData.window` returns undefined.

Replaces this with the following:
> let parsedData = JSON.parse(aData);
> Services.console.logStringMessage(aData.window, outerWindowID);
> if (parsedData.window && parsedData.window !== outerWindowID) {
>   return;
> }

And that's all there is to it! It works now for me with your VivaldiFox addon and these changes.

Comment 18

[Tim Nguyen :ntim]

Comment on attachment 8889170 [details]
Bug 1342712 - Allow scoping a theme per-window.

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

Comment 19

[Tim Nguyen :ntim]

Comment on attachment 8889170 [details]
Bug 1342712 - Allow scoping a theme per-window.

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

Comment 20

[Tim Nguyen :ntim]

Comment on attachment 8889170 [details]
Bug 1342712 - Allow scoping a theme per-window.

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

Comment 21

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

Comment on attachment 8889170 [details]
Bug 1342712 - Allow scoping a theme per-window.

https://reviewboard.mozilla.org/r/160268/#review170422

Looks good, nice and concise too.

::: toolkit/components/extensions/ext-theme.js:39
(Diff revision 2)
>     * Loads a theme by reading the properties from the extension's manifest.
>     * This method will override any currently applied theme.
>     *
>     * @param {Object} details Theme part of the manifest. Supported
>     *   properties can be found in the schema under ThemeType.
> +   * @param {Object} targetWindow The window to apply the theme to. Ommiting

s/Ommiting/Omitting/

::: toolkit/components/extensions/ext-theme.js:296
(Diff revision 2)
>              // WebExtensions using the Theme API will not have a theme defined
>              // in the manifest. Therefore, we need to initialize the theme the
>              // first time browser.theme.update is called.
>              this.theme = new Theme(extension.baseURI, extension.logger);
>            }
> -
> +          

nit, remove this whitespace

Comment 22

[Tim Nguyen :ntim]

Comment on attachment 8889170 [details]
Bug 1342712 - Allow scoping a theme per-window.

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

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8585560b64f1
Allow scoping a theme per-window. r=jaws

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8585560b64f1

Comment 25

[Will Bamberg [:wbamberg]]

I've updated the page: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/theme/update

Let me know if this covers it.

Comment 26

[Tim Nguyen :ntim]

Looks good to me, thanks!

Comment 27

[marius.santa]

Please provide a 57+ compatible webextension for testing purposes.

Comment 28

[Tim Nguyen :ntim]

(In reply to marius.santa from comment #27)
> Please provide a 57+ compatible webextension for testing purposes.

Hi Marius,

attachment 8913856 [details] is an extension that uses this API. Private windows should have a red UI.

Another extension is https://addons.mozilla.org/en-US/firefox/addon/containers-theme/ which matches the UI color with the current container.