Closed Bug 1347207 Opened 7 years ago Closed 5 years ago

Implement theme experiments

Categories

(WebExtensions :: Themes, enhancement, P5)

Product:
WebExtensions
Component:
Themes
Type:
enhancement

Tracking

(firefox63 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla63
Tracking Flags:
Tracking Status
firefox63 --- fixed

People

(Reporter: mikedeboer, Assigned: ntim)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: triaged [ntim-intern-project])

User Story

As a theme author I’d like to be able to propose new properties that will allow all theme authors to style different parts of the browser using the following API:

```json
{
  "manifest_version": 2,
  "name": "Moar Custom Icons",
  "description": "Example of a theme experiment",
  "version": "1.0",
  "applications": {
    "gecko": {
      "id": "moar-custom-icons@example.com",
      "strict_min_version": "57.0",
      "strict_max_version": "65.*"
    }
  },
  "icons": {
    "48": "icon.png",
    "96": "icon@2x.png"
  },
  "theme": {
    "colors": {
      "popup_affordance": [128,128,128]
    },
    "images": {
      "headerURL": "image.jpeg",
      "theme_toolbar": "toolbar.jpg"
    }
  },
  "theme_experiment": {
    "stylesheet": "style.css", // optional
    "colors": {
      "popup_affordance": "arrowpanel-dimmed"
    },
    "images": {
      "theme_toolbar": "toolbar-bgimage"
    },
    "properties": {
      "toolbar_image_alignment": "toolbar-bgalignment"
    }
  }
}
```

Attachments

(4 files)

Bug 1347207 - Implement theme_experiment manifest field.
59 bytes, text/x-review-board-request
jaws
: review+
Details
Whiteboard session at SF All Hands
1.54 MB, image/jpeg
Details
Bug 1347207 - Add and fix tests for theme loading warnings.
59 bytes, text/x-review-board-request
mixedpuppy
: review+
Details
theme_experiment.png
12.35 KB, image/png
Details 
      No description provided.
Blocks: 1347210
Priority: -- → P5
Talking with ntim about this and we think that the experimental CSS section would have better use if it allowed for specifying a CSS file that would get included instead of converting CSS to a JSON-friendly format.

The downside here is that it would make it harder for us to figure out what components theme authors are overriding, but we may also be able to learn these by analyzing popular experimental themes anyways, it would just be a little more work. The upside is that this would allow theme authors a better developer experience, and the previously suggested "selector" property-name was not required to coordinate with an existing schema so it's value was questionable.
User Story: (updated)
Assignee: nobody → ntim.bugs
I'm not sure what the intentions are for this feature but we've steadfastly avoided giving access to chrome css to extensions and I don't think we should start now.  There are certainly interesting legitimate uses but a malicious extension could easily abuse it as well.  For instance they could change the site identity or permissions indicators on particular sites.

If we add this section, AMO should *not* sign themes that include it.
This looks like the theme equivalent of WebExtensions Experiments, so I agree that they don't belong on AMO (nor they should be easy to distribute and install broadly).
The patch works, I'm planning to add tests tomorrow.
Before getting into code review, I'd like to discuss the high level idea first.
If this just exists to support the built-in compact themes, then I think its a bit over-engineered, lets just add the features those themes need but limit access to them to built-in themes if necessary...
(In reply to Andrew Swan [:aswan] from comment #6)
> Before getting into code review, I'd like to discuss the high level idea
> first.
> If this just exists to support the built-in compact themes, then I think its
> a bit over-engineered, lets just add the features those themes need but
> limit access to them to built-in themes if necessary...

This was part of the original engineering plan: https://docs.google.com/document/d/1ueD6V7aLLTuc1GAOxxQYcwl2HR-k62HHu3q8knTJ4FU/pub in the spirit of WebExtensions Experiments. It wasn't originally created to support the built-in themes, I just thought later-on this feature would be a nice way to support them.
Comment on attachment 8940885 [details]
Bug 1347207 - Implement theme_experiment manifest field.

Hi Tim, cool that you're picking this up!

I talked about this API with Jared as well and I noted that I'm not sold on the idea of injecting additional CSS into the browser. It just grants too many superpowers to extension authors - even though it only works on Nightly.

I think we want to make a more specific solution driven that may also solve the missing features problem you have to implement the light and dark themes using the Theming API.

I'd like to propose the following:

 * Don't use the Experimental API for the things we need for the Dark and Light themes to be fully supported, but instead add & implement properties that Mozilla-signed themes may use. Indeed, proprietary stuff like '-moz-mac-vibrant-xxx' is also something we can enable that way.
 ** The idea here is to extend the `load()` method in ext-theme.js with a call to a new `loadPropertiesWhenSigned()` which will check if the extension is in fact signed and validated and adds the whitelisted properties when defined to the theme data.
 * Implement the original experimental API idea using property maps, so that it's easier to parse and prune CSS statements *before* they are evaluated in the chrome context.

What do you think? We can also talk the plan through on IRC & Vidyo, of course!
Attachment #8940885 - Flags: review?(jaws)
Attachment #8940885 - Flags: review?(aswan)
mass move of existing themes bugs to new WebExtensions: Themes component
Component: WebExtensions: Frontend → WebExtensions: Themes
Unassigning to reflect real status.
Assignee: ntim.bugs → nobody
Assignee: nobody → ntim.bugs
Updated user story with what was discussed in SF with :mikedeboer
User Story: (updated)
User Story: (updated)
Product: Toolkit → WebExtensions
Whiteboard: triaged → triaged [ntim-intern-project]
We should make sure that -moz-binding is not allowed to be used by the CSS with this API.
Depends on: 1477708
Attachment #8940885 - Flags: review?(jaws)
Attachment #8940885 - Flags: review?(aswan)
Comment on attachment 8940885 [details]
Bug 1347207 - Implement theme_experiment manifest field.

https://reviewboard.mozilla.org/r/211142/#review267274


Code analysis found 5 defects in this patch:
 - 5 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/parent/ext-theme.js:68
(Diff revision 2)
> +    } else {
> -    this.load();
> +      this.load();
> -  }
> +    }
> +  }
> +
> +  /**

Error: Missing JSDoc @returns for function. [eslint: valid-jsdoc]

::: toolkit/components/extensions/parent/ext-theme.js:73
(Diff revision 2)
> +  /**
> +   * Check if the theme can run the theme experiment
> +   * Experiments are only allowed in system add-ons and on the dev channels.
> +   */
> +  async themeCanRunExperiment() {
> +    let addon = await AddonManager.getAddonByID(this.extension.id);

Error: 'AddonManager' is not defined. [eslint: no-undef]

::: toolkit/modules/LightweightThemeConsumer.jsm:236
(Diff revision 2)
> +    }
> +    if (active && experiment) {
> +      this._lastExperimentData = {};
> +      if (experiment.stylesheet) {
> +        let stylesheetAttr = `href="${experiment.stylesheet}" type="text/css"`;
> +        let stylesheet = doc.createProcessingInstruction("xml-stylesheet",

Error: 'stylesheet' is assigned a value but never used. [eslint: no-unused-vars]

::: toolkit/modules/LightweightThemeConsumer.jsm:236
(Diff revision 2)
> +    }
> +    if (active && experiment) {
> +      this._lastExperimentData = {};
> +      if (experiment.stylesheet) {
> +        let stylesheetAttr = `href="${experiment.stylesheet}" type="text/css"`;
> +        let stylesheet = doc.createProcessingInstruction("xml-stylesheet",

Error: 'doc' is not defined. [eslint: no-undef]

::: toolkit/modules/LightweightThemeConsumer.jsm:238
(Diff revision 2)
> +      this._lastExperimentData = {};
> +      if (experiment.stylesheet) {
> +        let stylesheetAttr = `href="${experiment.stylesheet}" type="text/css"`;
> +        let stylesheet = doc.createProcessingInstruction("xml-stylesheet",
> +          stylesheetAttr);
> +        this._doc.insertBefore(styleSheet, root);

Error: 'styleSheet' is not defined. [eslint: no-undef]
Comment on attachment 8940885 [details]
Bug 1347207 - Implement theme_experiment manifest field.

https://reviewboard.mozilla.org/r/211142/#review267276


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/parent/ext-theme.js:68
(Diff revision 3)
> +    } else {
> -    this.load();
> +      this.load();
> -  }
> +    }
> +  }
> +
> +  /**

Error: Missing JSDoc @returns for function. [eslint: valid-jsdoc]

::: toolkit/components/extensions/parent/ext-theme.js:73
(Diff revision 3)
> +  /**
> +   * Check if the theme can run the theme experiment
> +   * Experiments are only allowed in system add-ons and on the dev channels.
> +   */
> +  async themeCanRunExperiment() {
> +    let addon = await AddonManager.getAddonByID(this.extension.id);

Error: 'AddonManager' is not defined. [eslint: no-undef]

::: toolkit/modules/LightweightThemeConsumer.jsm:236
(Diff revision 3)
> +    }
> +    if (active && experiment) {
> +      this._lastExperimentData = {};
> +      if (experiment.stylesheet) {
> +        let stylesheetAttr = `href="${experiment.stylesheet}" type="text/css"`;
> +        let stylesheet = doc.createProcessingInstruction("xml-stylesheet",

Error: 'doc' is not defined. [eslint: no-undef]
Comment on attachment 8940885 [details]
Bug 1347207 - Implement theme_experiment manifest field.

https://reviewboard.mozilla.org/r/211142/#review267290


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/parent/ext-theme.js:68
(Diff revision 4)
> +    } else {
> -    this.load();
> +      this.load();
> -  }
> +    }
> +  }
> +
> +  /**

Error: Missing JSDoc @returns for function. [eslint: valid-jsdoc]

::: toolkit/components/extensions/parent/ext-theme.js:73
(Diff revision 4)
> +  /**
> +   * Check if the theme can run the theme experiment
> +   * Experiments are only allowed in system add-ons and on the dev channels.
> +   */
> +  async themeCanRunExperiment() {
> +    let addon = await AddonManager.getAddonByID(this.extension.id);

Error: 'AddonManager' is not defined. [eslint: no-undef]

::: toolkit/modules/LightweightThemeConsumer.jsm:236
(Diff revision 4)
> +    }
> +    if (active && experiment) {
> +      this._lastExperimentData = {};
> +      if (experiment.stylesheet) {
> +        let stylesheetAttr = `href="${experiment.stylesheet}" type="text/css"`;
> +        let stylesheet = doc.createProcessingInstruction("xml-stylesheet",

Error: 'doc' is not defined. [eslint: no-undef]
Comment on attachment 8940885 [details]
Bug 1347207 - Implement theme_experiment manifest field.

https://reviewboard.mozilla.org/r/211142/#review267294


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/parent/ext-theme.js:68
(Diff revision 5)
> +    } else {
> -    this.load();
> +      this.load();
> -  }
> +    }
> +  }
> +
> +  /**

Error: Missing JSDoc return type. [eslint: valid-jsdoc]
Comment on attachment 8940885 [details]
Bug 1347207 - Implement theme_experiment manifest field.

https://reviewboard.mozilla.org/r/211142/#review267320


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/parent/ext-theme.js:71
(Diff revision 6)
> +    } else {
> -    this.load();
> +      this.load();
> -  }
> +    }
> +  }
> +
> +  /**

Error: Missing JSDoc return type. [eslint: valid-jsdoc]
Status: NEW → ASSIGNED
Comment on attachment 8940885 [details]
Bug 1347207 - Implement theme_experiment manifest field.

https://reviewboard.mozilla.org/r/211142/#review267892

Looks good. Will need tests before this can land though (which you already know :) )

::: toolkit/components/extensions/parent/ext-theme.js:62
(Diff revision 6)
> +          if (experiment.stylesheet) {
> +            baseURI.resolve(experiment.stylesheet);
> +          }
> +          this.experiment = experiment;
> +        } else {
> +          logger.warn("This extension is not allowed to run theme experiments");

We should just throw an exception here and not load the extension.

::: toolkit/components/extensions/parent/ext-theme.js:64
(Diff revision 6)
> +          }
> +          this.experiment = experiment;
> +        } else {
> +          logger.warn("This extension is not allowed to run theme experiments");
> +        }
> +        this.load();

You can remove `this.load()` and then pull the `this.load()` outside of the `else` block below so it will always run.

::: toolkit/components/extensions/parent/ext-theme.js:373
(Diff revision 6)
> +    let {theme, theme_experiment} = manifest;
> +
> +    if (!theme && theme_experiment) {
> +      this.experiment = theme_experiment;
> +    } else {
> +      this.experiment = null;

should this be clearing theme_experiment too?

::: toolkit/modules/LightweightThemeConsumer.jsm:219
(Diff revision 6)
>        root.removeAttribute("lwthemefooter");
>  
> -    let contentThemeData = _getContentProperties(this._doc, active, aData);
> +    let contentThemeData = _getContentProperties(this._doc, active, theme);
>      Services.ppmm.sharedData.set(`theme/${this._winId}`, contentThemeData);
> +  },
> +  _setExperiment(active, experiment, properties) {

nit, put a blank line above this

::: toolkit/modules/LightweightThemeConsumer.jsm:235
(Diff revision 6)
> +      }
> +    }
> +    if (active && experiment) {
> +      this._lastExperimentData = {};
> +      if (experiment.stylesheet) {
> +        let stylesheetAttr = `href="${experiment.stylesheet}" type="text/css"`;

nb, this is okay since the schema restricts stylesheet to an ExtensionURL
Attachment #8940885 - Flags: review?(jaws) → review+
Comment on attachment 8940885 [details]
Bug 1347207 - Implement theme_experiment manifest field.

https://reviewboard.mozilla.org/r/211142/#review267900

::: toolkit/components/extensions/parent/ext-theme.js:78
(Diff revision 6)
> +   * Experiments are only allowed in system add-ons and on the dev channels.
> +   * @returns boolean true if experiments can be ran, false if not
> +   */
> +  async themeCanRunExperiment() {
> +    let addon = await AddonManager.getAddonByID(this.extension.id);
> +    return !AppConstants.RELEASE_OR_BETA || addon.isSystem;

I haven't read the rest of the patch but if this allows themes to insert arbitrary css into chrome documents, this is not strong enough, there should be a separate pref defaulting to off that needs to be flipped to enable this.
Ni? about using !RELEASE_OR_BETA or MOZ_ALLOW_LEGACY_EXTENSIONS.
Flags: needinfo?(jaws)
Comment on attachment 8940885 [details]
Bug 1347207 - Implement theme_experiment manifest field.

https://reviewboard.mozilla.org/r/211142/#review268986


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python/etc)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/parent/ext-theme.js:55
(Diff revision 7)
> +      this.lwtStyles.experimental = {
> +        colors: {},
> +        images: {},
> +        properties: {},
> +      };
> +      const {baseURI, logger} = this.extension;

Error: 'logger' is assigned a value but never used. Allowed unused vars must match /^console$/. [eslint: no-unused-vars]
Comment on attachment 8940885 [details]
Bug 1347207 - Implement theme_experiment manifest field.

https://reviewboard.mozilla.org/r/211142/#review268988


Code analysis found 2 defects in this patch:
 - 2 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python/etc)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/parent/ext-theme.js:49
(Diff revision 8)
>      this.lwtStyles = {
>        icons: {},
>      };
>  
> +    if (experiment) {
> +      const canRunExperiment = !AppConstants.RELEASE_OR_BETA && 

Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

::: toolkit/components/extensions/parent/ext-theme.js:57
(Diff revision 8)
> +        this.lwtStyles.experimental = {
> +          colors: {},
> +          images: {},
> +          properties: {},
> +        };
> +        const {baseURI, logger} = this.extension;

Error: 'logger' is assigned a value but never used. Allowed unused vars must match /^console$/. [eslint: no-unused-vars]
(In reply to Tim Nguyen :ntim from comment #28)
> Ni? about using !RELEASE_OR_BETA or MOZ_ALLOW_LEGACY_EXTENSIONS.

Using AppConstants.MOZ_ALLOW_LEGACY_EXTENSIONS is fine with me though you will to also check the `extensions.legacy.enabled` pref to satisfy aswan's request that an extra pref would be needed.
Flags: needinfo?(jaws)
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/91a30d78c074
Implement theme_experiment manifest field. r=jaws
ESlint failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=193006227&repo=autoland&lineNumber=260

[task 2018-08-09T11:49:03.547Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/parent/ext-theme.js:194:5 | Block must not be padded by blank lines. (padded-blocks)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js:17:10 | Missing trailing comma. (comma-dangle)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js:28:10 | Missing trailing comma. (comma-dangle)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js:29:8 | Missing trailing comma. (comma-dangle)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js:76:10 | Missing trailing comma. (comma-dangle)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js:77:8 | Missing trailing comma. (comma-dangle)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js:89:10 | Missing trailing comma. (comma-dangle)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js:103:6 | Trailing spaces not allowed. (no-trailing-spaces)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js:104:1 | Trailing spaces not allowed. (no-trailing-spaces)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js:164:15 | Trailing spaces not allowed. (no-trailing-spaces)
[task 2018-08-09T11:54:14.595Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js:181:6 | Missing trailing comma. (comma-dangle)
[taskcluster 2018-08-09 11:54:15.139Z] === Task Finished ===
[taskcluster 2018-08-09 11:54:15.139Z] Unsuccessful task run with exit code: 1 completed in 568.497 seconds
Flags: needinfo?(ntim.bugs)
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/421c12a2837b
Implement theme_experiment manifest field. r=jaws
No longer blocks: 1347210
Summary: Implement 'experimental' theme manifest section → Implement theme experiments
Backed out for failing browser chrome at toolkit/components/extensions/test/browser/browser_ext_themes_experiment.js

Push that caused the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=421c12a2837ba777d295a891ff4de9637792011e

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=193029357&repo=autoland&lineNumber=15419

Backout: https://hg.mozilla.org/integration/autoland/rev/249570f5e1ba0dcddb6b0b562a28a03a38a715ef
Flags: needinfo?(ntim.bugs)
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/20d0116ece9a
Implement theme_experiment manifest field. r=jaws
Flags: needinfo?(ntim.bugs)
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/65a6a118cb1a
acked out changeset 20d0116ece9a for failing xpcshell at  xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_manifest_themes.js on a CLOSED TREE
Backed out for failing xpcshell at xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_manifest_themes.js

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=20d0116ece9a1b88eceaf1876aabd2a162ca5496

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=193064474&repo=autoland&lineNumber=7496

Backout: https://hg.mozilla.org/integration/autoland/rev/65a6a118cb1a7bbe4c2af3ee41e89eb6704f48bc
Flags: needinfo?(ntim.bugs)
Comment on attachment 8998936 [details]
Bug 1347207 - Add and fix tests for theme loading warnings.

https://reviewboard.mozilla.org/r/261662/#review269170
Attachment #8998936 - Flags: review?(mixedpuppy) → review+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/98010ec67528
Implement theme_experiment manifest field. r=jaws
https://hg.mozilla.org/integration/autoland/rev/56a97a3eee82
Add and fix tests for theme loading warnings. r=mixedpuppy
https://hg.mozilla.org/mozilla-central/rev/20d0116ece9a
https://hg.mozilla.org/mozilla-central/rev/65a6a118cb1a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Flags: needinfo?(ntim.bugs)
Keywords: dev-doc-needed
Depends on: 1482870
Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe-verify -“ flag.

Thanks!
Flags: needinfo?(ntim.bugs)
Flags: needinfo?(ntim.bugs) → qe-verify-

Can I confirm the documentation updates required:

  • in summary: this is a new manifest key theme_experiment providing for the properties: stylesheet, images, and properties. It enables developers to do two things:

  • override page CSS and therefore align look of all pages with the browser theme.

  • override the images used for icons in the browser UI and apply properties to those images, enabling the developer to more deeply customize the UI.
    It is supported on desktop Firefox only from version 63.

  • manifest keys pages: add a new page for theme_experiment with the following content:

Type: Object
Mandatory: No
Example:

"theme_experiment": {
"stylesheet": "style.css", // optional
"colors": {
"popup_affordance": "arrowpanel-dimmed"
},
"images": {
"theme_toolbar": "toolbar-bgimage"
},
"properties": {
"toolbar_image_alignment": "toolbar-bgalignment"
}
}

Theme experiments enable to features:

  • overriding the CSS of pages so that the presentation of pages more closely matches the theme applied to the browser UI.
  • specification of new images to use as content within Firefox UI components, thus enabling the UI to be more deeply customized. MSyntax

The theme key is an object that takes the following properties:

Name Type Description
stylesheet Object Provides for the optional specification of a stylesheet and stylesheet elements. The stylesheet elements are either added to the specified stylesheet or used as stylesheet overrides.
images Object Provides for the specification of images to override theme elements. A list of theme element names is available (where?)
properties Object Provides for the specification of additional properties for theme elements. The listed theme properties is available (where?)
  • browser compatibility data: add a compatibility file for theme_experiment and include the properties stylesheet, images, and properties indicating their availability from Firefox for desktop version 63.
Flags: needinfo?(ntim.bugs)

Sorry for not providing much detail in this bug. Here's a summary of how it works:

This feature allows defining new theme properties for styling parts of the Firefox interface that the built-in properties don't cover. One very important thing to note is that the feature is limited to Developer Edition and Nightly channels behind the extensions.legacy.enabled preference.

The stylesheet is injected inside the Firefox interface and lets you define CSS variables that can then be mapped in the colors, images and properties objects to new theme properties.

For example, I could have a style.css file which has:

#reload-button {
  fill: var(--reload-button-color);
}

(the #reload-button ID references to Firefox's internal code, and --reload-button-color is a name you make up).

and then in the manifest.json file, I could use:

"theme_experiment": {
  "stylesheet": "style.css",
  "colors": {
    "reload_button": "--reload-button-color"
  }
}

and you can use reload_button as any other theme property:

"theme": {
  "colors": {
    "reload_button": "orange"
  }
}

which would make the reload icon color orange. You can also use this property in browser.theme.update() like other property.

images and properties work pretty similarly to colors.

Without stylesheet, you can still use this feature to map internal CSS variables that Firefox defines (like --arrowpanel-dimmed) to new theme properties. There isn't an exhaustive list of those anywhere, since the feature is mostly about defining new theme properties using Firefox internals.

Though I think most of the usefulness from this feature would come from the stylesheet feature, given that it can inject arbitrary CSS into the Firefox UI and hence have a lot of power.

Note that this feature could change at any time (though it hasn't so far), since it's experimental and only exposed to Nightly/Developer Edition.

Please let me know if you have any questions :)

Flags: needinfo?(ntim.bugs)

Hi Tim,

I've attached my work in progress writeup below, but also have some questions/requests:

  1. given that you can't use this to release a theme, what is the purpose? Is it simply to give developers the option to play around? could it lead to a feature For the release version of Firefox?
  2. why wasn't this done by providing a set of hidden/experimental properties in theme?
  3. I'm still somewhat unclear what the advantage of defining the stylesheet given that all you appear to do is map a Firefox UI component ID to an arbitrary name which in there then is in turn mapped to an arbitrary name for a key view using theme. Why isn't it easier simply to map the Firefox UI component ID to the key property in theme_experiment ?
  4. If you don't make use of the stylesheet, in your example would you define "colors": {"reload_button": "#reload-button"}?
  5. Where can I find a list of/lookup/discover the Firefox UI component ID?

Thanks!

<table>
<tbody>
<tr>
<td>Type</td>
<td>Object</td>
</tr>
<tr>
<td>Mandatory</td>
<td>No</td>
</tr>
<tr>
<td>Example</td>
<td><pre>"theme_experiment": {
"stylesheet": "style.css",
"colors": {
"popup_affordance": "arrowpanel-dimmed"
},
"images": {
"theme_toolbar": "toolbar-bgimage"
},
"properties": {
"toolbar_image_alignment": "toolbar-bgalignment"
}
} </pre>
</td>
</tr>
</tbody>
</table>

This key enables new theme key properties for the Firefox interface to be defined. This is done by either creating a stylesheet that provides mapping between the Firefox internal ID for a UI element feature and an arbitrary … ( to be completed)

The stylesheet is injected inside the Firefox interface and lets you define CSS variables that can then be mapped in the colors, images and properties objects to new theme properties.

Without stylesheet, you can still use this feature to map internal CSS variables that Firefox defines (like --arrowpanel-dimmed) to new theme properties. There isn't an exhaustive list of those anywhere, since the feature is mostly about defining new theme properties using Firefox internals.

Note: This key is only available for use in Firefox Developer Edition and Firefox Nightly channels and requires the extensions.legacy.enabled preference to be enabled.

** Warning**: This feature is experimental and could be subject to change.

Syntax

The theme key is an object that takes the following properties:

Name Type Description
stylesheet String Name of the stylesheet providing mapping information for images and properties.
images Object Details of mappings between UI element Firefox internal IDs for images and property names to be used in themes.
colors Object Details of mappings between UI element Firefox internal IDs for colors and property names to be used in themes.
properties Object Details of mappings between UI element Firefox internal IDs for properties and property names to be used in themes.

A list of UI element Firefox internal IDs is available (where?)

Examples

This example uses a stylesheet named style.css to provide the ability to define a color for the browser reload button in the theme key.

The stylesheet defines:

#reload-button {
  fill: var(--reload-button-color);
}

where #reload-button is the Firefox internal ID for the reload button and --reload-button-color is an arbitrary name.

In the manifest.json file, --reload-button-color is then mapped to the name to be used in the theme colors property:

"theme_experiment": {
  "stylesheet": "style.css",
  "colors": {
    "reload_button": "--reload-button-color"
  }
}

Then reload_button is used in the same way as any other theme property:

"theme": {
  "colors": {
    "reload_button": "orange"
  }
}

This has the effect of making the reload icon orange.

(add image)

This property can also be used in browser.theme.update(). images and properties work in a similar way to colors.

Flags: needinfo?(ntim.bugs)

given that you can't use this to release a theme, what is the purpose? Is it simply to give developers the option to play around? could it lead to a feature For the release version of Firefox?

Same purpose as WebExtension experiments, propose new theming API features for inclusion in Firefox.

why wasn't this done by providing a set of hidden/experimental properties in theme?

We wouldn't be able to cover everyone's needs, so it's better that extension developers define the experiment themselves.

I'm still somewhat unclear what the advantage of defining the stylesheet given that all you appear to do is map a Firefox UI component ID to an arbitrary name which in there then is in turn mapped to an arbitrary name for a key view using theme. Why isn't it easier simply to map the Firefox UI component ID to the key property in theme_experiment ?

The mapping maps a key property in theme_experiment to a Firefox UI CSS variable, to a not UI element/component ID. The stylesheet lets you define those Firefox UI CSS variables.

Also, CSS variables can be used in a more creative way then the example I showed above, a more crazy example would be:

#reload-button {
  fill: var(--reload-button-color);
}

#reload-button:hover {
  background-color: var(--reload-button-color);
  fill: white;
}

If you don't make use of the stylesheet, in your example would you define "colors": {"reload_button": "#reload-button"}?

That simply wouldn't work. The value needs to be defined CSS variable, hence why the stylesheet is useful, since it allows to define new CSS variables.
If you don't make use of the stylesheet, you can only map to CSS variables (not component IDs) that Firefox already defines internally. There's unfortunately not centralized place to find all the CSS vars, but by inspecting through the browser toolbox, it's possible to find them.

https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox

Where can I find a list of/lookup/discover the Firefox UI component ID?

You can inspect the Firefox interface using the Browser Toolbox.

Flags: needinfo?(ntim.bugs)
Attached image theme_experiment.png —

Hi Tim, thanks for your feedback. Here is my final draft for the MDN content. do you have any further comments?
Type Object
Mandatory No
Example "theme_experiment": { <br> "stylesheet":<br>"style.css",<br>"colors": {<br> "popup_affordance": "arrowpanel-dimmed" <br> },<br> "images":<br>{<br>"theme_toolbar": "toolbar-bgimage"<br>},<br>"properties": {<br>"toolbar_image_alignment":<br>"toolbar-bgalignment"<br>}<br>}

This key enables experimental theme key properties for the Firefox interface to be defined. These experiments are a precursor to proposing new theme features for inclusion in Firefox. Experimentation is done by:

  • creating a stylesheet that defines mappings between internal IDs for Firefox UI element features and arbitrary CSS variables. The CSS variables are then mapped in the colors, images and properties objects to new theme key properties.
  • (without a stylesheet) using colors, images and properties to map internal Firefox CSS variables, such as --arrowpanel-dimmed to new theme key properties. This option limits experimentation to UI components that are associated with a inbuilt CSS variable.

To discover the internal IDs for Firefox UI elements or internal Firefox CSS variables use the browser toolbox.

Note: This key is only available for use in Firefox Developer Edition and Firefox Nightly channels and requires the extensions.legacy.enabled preference to be enabled.

Warning: This feature is experimental and could be subject to change.

Syntax

The theme key is an object that takes the following properties:

Name Type Description
stylesheet String Name of a stylesheet providing mapping of Firefox UI element feature internal IDs to CSS variables.
images Object Mappings of CSS variables (as defined in Firefox or by the stylesheet defined instylesheet) to images property names for use in themes.
colors Object Mappings of CSS variables (as defined in Firefox or by the stylesheet defined instylesheet) to colors property names for use in themes.
properties Object Mappings of CSS variables (as defined in Firefox or by the stylesheet defined instylesheet) to properties property names for use in themes.

Examples

This example uses a stylesheet named style.css to provide the ability to define a color for the browser reload button in the themekey.

The stylesheet defines:

#reload-button {
  fill: var(--reload-button-color);
}

where #reload-button is the Firefox internal ID for the reload button and --reload-button-color is an arbitrary name.

In the manifest.json file, --reload-button-coloris then mapped to the name to be used in the theme``colorsproperty:

"theme_experiment": {
  "stylesheet": "style.css",
  "colors": {
    "reload_button": "--reload-button-color"
  }
}

Then reload_button is used in the same way as any other theme property:

"theme": {
  "colors": {
    "reload_button": "reload_button"
  }
}

This has the effect of making the reload icon orange.

(image)

This property can also be used in browser.theme.update(). images and properties work in a similar way to colors.

Flags: needinfo?(ntim.bugs)

Looks great, thanks!

Some small nits:

  • "internal IDs" should probably "internal CSS selectors" given that you can use other CSS selectors too.
  • In the table, I'm not sure why themes is a code quote, I think it should just be a normal word since it's not referring to any code.

needinfo'ing Jared/Mike if they have any extra feedback.

Flags: needinfo?(ntim.bugs)
Flags: needinfo?(mdeboer)
Flags: needinfo?(jaws)

Thanks Tim, I'll start preparing the content for MDN pending feedback from Jared and Mike.

Hi Jared/Mike, do you have any feedback on the proposed content, or can we go ahead and publish?

FYI Tim, Jered, and Mike new pages now on MDN: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/theme_experiment If you have any further comments, please let me know or edit the page directly. Thank you!

Flags: needinfo?(mdeboer)
Flags: needinfo?(jaws)
You need to log in before you can comment on or make changes to this bug.