Bug 1343921 implemented an API to add icons to webExtensions. This is like LW-Themes with the ability to theme the button icons.

Magnus, I'm asking now only for feedback awaiting an answer from Mike if it would be okay to add our entities to the toolkit theme scheme. To work it needs both patches for c-c and m-c.

Mike, TB needs to add entities to the theme scheme. Would this be okay? There a lot more than FX has because TB has also a lot more buttons. Or does a possibility exist to add them during runtime? Or could it be implemented that these schemes are placed in browser and mail and toolkit loads them during build from there?

This quickly made webExtension works on FX and TB. Thanks to bug 1339131 it does no more fail when unknown entities are found. To test load about:debugging in TB's home page and with "Load Temporary Add-on" the XPI can be loaded temporary. Before, the prefs extensions.webextensions.themes.enabled and extensions.webextensions.themes.icons.enabled need to be set to true.

Hi Richard, cool to see you working on this! I do think, however it might be a bit too soon, because we're still working on the framework itself. For example, we don't think that having the list of icons live in a pref is ideal; we'll probably change it to be in a .js file called 'ThemeData.js', which'll contain icon names and many other properties in the near future. What do you think of: ```js let themeData = null; try { themeData = Cu.import("resource:///modules/ThemeData.js", {}).ThemeData; } catch (ex) { Cu.reportError("No Theme details could be found, which will severely limit your theming options."); } console.dir(themeData.icons); // '["icon1", "icon2", ...]' ``` ?

Also, please be aware that we'll put in a hard requirement for SVG icons to be used as icons to be able to support different DPI settings.

Mike, thank you for checking this. Then I'm waiting until it's in a final stage. (In reply to Mike de Boer [:mikedeboer] from comment #4) > What do you think of: > ```js > let themeData = null; > try { > themeData = Cu.import("resource:///modules/ThemeData.js", {}).ThemeData; > } catch (ex) { > Cu.reportError("No Theme details could be found, which will severely limit > your theming options."); > } > > console.dir(themeData.icons); // '["icon1", "icon2", ...]' > ``` > > ? This sounds good. (In reply to Mike de Boer [:mikedeboer] from comment #5) > Also, please be aware that we'll put in a hard requirement for SVG icons to > be used as icons to be able to support different DPI settings. Then I embed the bitmap in a SVG. ;) Joke aside, the extension was only quickly made to see a first result. My bitmaps don't look really good in the FX AppMenu panel.

(In reply to Mike de Boer [:mikedeboer] from comment #4) > Hi Richard, cool to see you working on this! I do think, however it might be > a bit too soon, because we're still working on the framework itself. > For example, we don't think that having the list of icons live in a pref is > ideal; we'll probably change it to be in a .js file called 'ThemeData.js', > which'll contain icon names and many other properties in the near future. > > What do you think of: > ```js > let themeData = null; > try { > themeData = Cu.import("resource:///modules/ThemeData.js", {}).ThemeData; > } catch (ex) { > Cu.reportError("No Theme details could be found, which will severely limit > your theming options."); > } > > console.dir(themeData.icons); // '["icon1", "icon2", ...]' > ``` > > ? Mike, are you still planning to do above?

Hi Richard, thanks for the ping! We've opted for defining the icons statically in theme.json, in the icons section. Please feel free to add icon identifiers there that are TB specific (jaws or I can review them just fine). In browser/base/content/theme-vars.inc.css you'll find how we activate the icons using CSS variables with same name on the root element. If you follow that example for TB windows, it *should* work. The activating mechanism (JS) is in LightweightThemeConsumer.jsm. Also remember to set the pref 'extensions.webextensions.themes.icons.enabled' to `true`.

This is the part for M-C which defines the elements to style.

C-C part updated to tip.

Comment on attachment 8871377 [details] [diff] [review] webExtIcons-m-c.patch, checked-in comment 17 Review of attachment 8871377 [details] [diff] [review]: ----------------------------------------------------------------- LGTM!

Aryx, please can you land the webExtIcons-m-c.patch in M-I and leave the bug open for the TB patch?

Not sure if Aryx around. Carsten, could you land the webExtIcons-m-c.patch and leave the bug open?

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6f0b91f39cf7 Add TB items to theme.json shema. r=mikedeboer

landed on m-i so it get merged to m-c at some point today

Thank you, Carsten.

This patch adds the webextension themes on Daily like bug 1341722 does. To test the patch you can use the webExt theme I attached in this bug. The pref extensions.webextensions.themes.icons.enabled needs to be set to true. Then the best is, you load about:debugging in TB's home page and with "Load Temporary Add-on" the XPI can be loaded temporary.

Bug 1341722 enabled the pref for all channels. I'll follow this change with the next patch or before landing.

This extension is now normally installable. You need only to enable extensions.webextensions.themes.icons.enabled to see the icons. Some icons are already SVG ones to test also this.

Comment on attachment 8873160 [details] [diff] [review] webExtIcons-c-c.patch Review of attachment 8873160 [details] [diff] [review]: ----------------------------------------------------------------- Pretty sweet, thx! r=mkmelin

https://hg.mozilla.org/comm-central/rev/167a8565ea427bfdd897a1f3835b075a7091a150 https://hg.mozilla.org/comm-central/rev/b6669e74cafa0fba300b034dcbb7f7b832a812a7 - backout https://hg.mozilla.org/comm-central/rev/4f6639dc0572bc3dc0f08c60f355d1ea2448355b - landed again with bug number.