Closed
Bug 1342712
Opened 8 years ago
Closed 7 years ago
Allow browser themes to be scoped to a specific window or active tab (like Vivaldi / VivaldiFox)
Categories
(WebExtensions :: Untriaged, defect, P5)
WebExtensions
Untriaged
Tracking
(firefox54 affected, firefox57 fixed)
RESOLVED
FIXED
mozilla57
People
(Reporter: callahad, Assigned: ntim)
References
(Depends on 2 open bugs)
Details
(Keywords: dev-doc-complete, DevAdvocacy, Whiteboard: [vivaldifox-blockers][design-decision-approved], triaged)
Attachments
(3 files)
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?
Reporter | ||
Comment 1•8 years ago
|
||
Comment 2•8 years ago
|
||
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.
Reporter | ||
Comment 4•8 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•8 years ago
|
||
potentially requiring front end support if doesn't exist. talk about in theming.
Flags: needinfo?(mwein)
Whiteboard: [design-decision-needed], triaged
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(mwein)
Resolution: --- → DUPLICATE
Reporter | ||
Comment 7•8 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)
Comment 8•7 years ago
|
||
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•7 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•7 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•7 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.
Updated•7 years ago
|
Flags: needinfo?(mwein)
Updated•7 years ago
|
Priority: -- → P5
Whiteboard: [design-decision-needed], triaged → [design-decision-approved], triaged
Assignee | ||
Comment 12•7 years ago
|
||
How about an optional parameter to browser.theme.update() called tabId ?
Comment hidden (mozreview-request) |
Assignee | ||
Comment 15•7 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 16•7 years ago
|
||
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•7 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.
Updated•7 years ago
|
Attachment #8889170 -
Flags: feedback?(jaws) → feedback+
Updated•7 years ago
|
Assignee: nobody → ntim.bugs
Status: REOPENED → ASSIGNED
Flags: needinfo?(matthewjwein)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 21•7 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•7 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•7 years ago
|
Blocks: themingapi
Comment 24•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8585560b64f1
Status: ASSIGNED → RESOLVED
Closed: 8 years ago → 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Updated•7 years ago
|
Whiteboard: [design-decision-approved], triaged → [vivaldifox-blockers][design-decision-approved], triaged
Updated•7 years ago
|
Keywords: dev-doc-needed
Comment 25•7 years ago
|
||
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)
Updated•7 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Comment 27•7 years ago
|
||
Please provide a 57+ compatible webextension for testing purposes.
Flags: needinfo?(dan.callahan)
Assignee | ||
Comment 28•7 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•7 years ago
|
Updated•6 years ago
|
Product: Toolkit → WebExtensions
Reporter | ||
Updated•4 years ago
|
Flags: needinfo?(dan.callahan)
You need to log in
before you can comment on or make changes to this bug.
Description
•