Closed Bug 1342712 Opened 3 years ago Closed 3 years ago

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

Categories

(WebExtensions :: Untriaged, defect, P5)

defect

Tracking

(firefox54 affected, firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox54 --- affected
firefox57 --- fixed

People

(Reporter: callahad, Assigned: ntim, NeedInfo)

References

(Depends on 2 open bugs)

Details

(Keywords: dev-doc-complete, DevAdvocacy, Whiteboard: [vivaldifox-blockers][design-decision-approved], triaged)

Attachments

(3 files)

Attached image 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?
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.
dan, does matts' reply answer your question?
Flags: needinfo?(dan.callahan)
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)
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: 3 years ago
Flags: needinfo?(mwein)
Resolution: --- → DUPLICATE
Duplicate of bug: 1320585
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#
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.
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
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
How about an optional parameter to browser.theme.update() called tabId ?
or windowId, which is the real blocker for my add-on.
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 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.
Assignee: nobody → ntim.bugs
Status: REOPENED → ASSIGNED
Flags: needinfo?(matthewjwein)
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+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8585560b64f1
Allow scoping a theme per-window. r=jaws
Blocks: themingapi
https://hg.mozilla.org/mozilla-central/rev/8585560b64f1
Status: ASSIGNED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Depends on: 1387737
Depends on: 1388010
Whiteboard: [design-decision-approved], triaged → [vivaldifox-blockers][design-decision-approved], triaged
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)
Looks good to me, thanks!
Flags: needinfo?(ntim.bugs)
Please provide a 57+ compatible webextension for testing purposes.
Flags: needinfo?(dan.callahan)
(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.
Depends on: 1404855
Blocks: themingapi-framework
No longer blocks: themingapi
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.