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

RESOLVED FIXED in Firefox 57

Status

defect
P5
normal
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: callahad, Assigned: ntim, NeedInfo)

Tracking

(Depends on 2 bugs, Blocks 1 bug, {dev-doc-complete, DevAdvocacy})

Trunk
mozilla57
Dependency tree / graph

Firefox Tracking Flags

(firefox54 affected, firefox57 fixed)

Details

(Whiteboard: [vivaldifox-blockers][design-decision-approved], triaged)

Attachments

(3 attachments)

Reporter

Description

2 years ago
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?
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

2 years ago
dan, does matts' reply answer your question?
Flags: needinfo?(dan.callahan)
Reporter

Comment 4

2 years ago
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.
Flags: needinfo?(dan.callahan)

Comment 5

2 years ago
potentially requiring front end support if doesn't exist.  talk about in theming.
Flags: needinfo?(mwein)
Whiteboard: [design-decision-needed], triaged
Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(mwein)
Resolution: --- → DUPLICATE
Duplicate of bug: 1320585
Reporter

Comment 7

2 years ago
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.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Summary: Allow per-window or per-tab theme differences (like Vivaldi / VivaldiFox) → Allow browser themes to be scoped to a specific window or active tab (like Vivaldi / VivaldiFox)
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#
Assignee

Comment 9

2 years ago
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.
Assignee

Comment 10

2 years ago
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
Reporter

Comment 11

2 years ago
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.
Flags: needinfo?(mwein)
Priority: -- → P5
Whiteboard: [design-decision-needed], triaged → [design-decision-approved], triaged
Assignee

Comment 12

2 years ago
How about an optional parameter to browser.theme.update() called tabId ?
Assignee

Comment 13

2 years ago
or windowId, which is the real blocker for my add-on.
Comment hidden (mozreview-request)
Assignee

Comment 15

2 years ago
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!
Attachment #8889170 - Flags: feedback?(mdeboer)
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!
Attachment #8889170 - Flags: feedback?(mdeboer) → feedback?(jaws)

Comment 17

2 years ago
mozreview-review
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.
Attachment #8889170 - Flags: feedback?(jaws) → feedback+
Assignee: nobody → ntim.bugs
Status: REOPENED → ASSIGNED
Flags: needinfo?(matthewjwein)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 21

2 years ago
mozreview-review
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
Attachment #8889170 - Flags: review?(jaws) → review+
Comment hidden (mozreview-request)

Comment 23

2 years ago
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8585560b64f1
Allow scoping a theme per-window. r=jaws
Assignee

Updated

2 years ago
Blocks: themingapi
https://hg.mozilla.org/mozilla-central/rev/8585560b64f1
Status: ASSIGNED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57

Updated

2 years ago
Depends on: 1387737

Updated

2 years ago
Depends on: 1388010
Assignee

Updated

2 years ago
Whiteboard: [design-decision-approved], triaged → [vivaldifox-blockers][design-decision-approved], triaged

Updated

2 years ago
Depends on: 1404496
I've updated the page: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/theme/update

Let me know if this covers it.
Flags: needinfo?(ntim.bugs)
Assignee

Comment 26

2 years ago
Looks good to me, thanks!
Flags: needinfo?(ntim.bugs)

Comment 27

2 years ago
Please provide a 57+ compatible webextension for testing purposes.
Flags: needinfo?(dan.callahan)
Assignee

Comment 28

2 years ago
(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.
Assignee

Updated

2 years ago
Depends on: 1404855
Assignee

Updated

2 years ago
Blocks: themingapi-framework
No longer blocks: themingapi

Updated

Last year
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.