Bug 1413144 - Make accentcolor and textcolor optional. r=jaws draft
authorTim Nguyen <ntim.bugs@gmail.com>
Tue, 10 Jul 2018 13:27:55 +0100
changeset 816916 833dcbb8c21a930e7b09119545ce623664237528
parent 816883 e41340c3ceef0dc67dda503f3facc1a86cc49614
push id115876
push userbmo:ntim.bugs@gmail.com
push dateWed, 11 Jul 2018 14:52:20 +0000
reviewersjaws
bugs1413144
milestone63.0a1
Bug 1413144 - Make accentcolor and textcolor optional. r=jaws MozReview-Commit-ID: 3jERl4H9vcv
browser/base/content/browser-compacttheme.js
toolkit/components/extensions/parent/ext-theme.js
toolkit/components/extensions/test/browser/browser_ext_themes_lwtsupport.js
toolkit/modules/LightweightThemeConsumer.jsm
toolkit/mozapps/extensions/LightweightThemeManager.jsm
--- a/browser/base/content/browser-compacttheme.js
+++ b/browser/base/content/browser-compacttheme.js
@@ -33,20 +33,20 @@ var CompactTheme = {
 
     if (this.isThemeCurrentlyApplied) {
       this._toggleStyleSheet(true);
     }
   },
 
   observe(subject, topic, data) {
     if (topic == "lightweight-theme-styling-update") {
-      let newTheme = JSON.parse(data);
-      if (newTheme && (
-          newTheme.id == "firefox-compact-light@mozilla.org" ||
-          newTheme.id == "firefox-compact-dark@mozilla.org")) {
+      let { theme } = JSON.parse(data) || {};
+      if (theme && (
+          theme.id == "firefox-compact-light@mozilla.org" ||
+          theme.id == "firefox-compact-dark@mozilla.org")) {
         // We are using the theme ID on this object instead of always referencing
         // LightweightThemeManager.currentTheme in case this is a preview
         this._toggleStyleSheet(true);
       } else {
         this._toggleStyleSheet(false);
       }
 
     }
--- a/toolkit/components/extensions/parent/ext-theme.js
+++ b/toolkit/components/extensions/parent/ext-theme.js
@@ -58,56 +58,50 @@ class Theme {
    * 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.
    */
   load(details) {
     this.details = details;
 
-    if (this.windowId) {
-      this.lwtStyles.window = getWinUtils(
-        windowTracker.getWindow(this.windowId)).outerWindowID;
-    }
-
     if (details.colors) {
       this.loadColors(details.colors);
     }
 
     if (details.images) {
       this.loadImages(details.images);
     }
 
     if (details.icons) {
       this.loadIcons(details.icons);
     }
 
     if (details.properties) {
       this.loadProperties(details.properties);
     }
 
-    // Lightweight themes require accentcolor and textcolor to be defined.
-    if (this.lwtStyles.accentcolor &&
-        this.lwtStyles.textcolor) {
-      if (this.windowId) {
-        windowOverrides.set(this.windowId, this);
-      } else {
-        windowOverrides.clear();
-        defaultTheme = this;
-      }
-      onUpdatedEmitter.emit("theme-updated", this.details, this.windowId);
+    let lwtData = {
+      theme: this.lwtStyles,
+    };
 
-      LightweightThemeManager.fallbackThemeData = this.lwtStyles;
-      Services.obs.notifyObservers(null,
-                                   "lightweight-theme-styling-update",
-                                   JSON.stringify(this.lwtStyles));
+    if (this.windowId) {
+      lwtData.window =
+        getWinUtils(windowTracker.getWindow(this.windowId)).outerWindowID;
+      windowOverrides.set(this.windowId, this);
     } else {
-      this.logger.warn("Your theme doesn't include one of the following required " +
-        "properties: 'headerURL', 'accentcolor' or 'textcolor'");
+      windowOverrides.clear();
+      defaultTheme = this;
     }
+    onUpdatedEmitter.emit("theme-updated", this.details, this.windowId);
+
+    LightweightThemeManager.fallbackThemeData = this.lwtStyles;
+    Services.obs.notifyObservers(null,
+                                 "lightweight-theme-styling-update",
+                                 JSON.stringify(lwtData));
   }
 
   /**
    * Helper method for loading colors found in the extension's manifest.
    *
    * @param {Object} colors Dictionary mapping color properties to values.
    */
   loadColors(colors) {
@@ -291,43 +285,33 @@ class Theme {
           this.lwtStyles.backgroundsTiling = tiling.join(",");
           break;
         }
       }
     }
   }
 
   static unload(windowId) {
-    let lwtStyles = {
-      headerURL: "",
-      accentcolor: "",
-      accentcolorInactive: "",
-      additionalBackgrounds: "",
-      backgroundsAlignment: "",
-      backgroundsTiling: "",
-      textcolor: "",
-      icons: {},
+    let lwtData = {
+      theme: null,
     };
 
     if (windowId) {
-      lwtStyles.window = getWinUtils(windowTracker.getWindow(windowId)).outerWindowID;
+      lwtData.window = getWinUtils(windowTracker.getWindow(windowId)).outerWindowID;
       windowOverrides.set(windowId, emptyTheme);
     } else {
       windowOverrides.clear();
       defaultTheme = emptyTheme;
     }
     onUpdatedEmitter.emit("theme-updated", {}, windowId);
 
-    for (let icon of ICONS) {
-      lwtStyles.icons[`--${icon}--icon`] = "";
-    }
     LightweightThemeManager.fallbackThemeData = null;
     Services.obs.notifyObservers(null,
                                  "lightweight-theme-styling-update",
-                                 JSON.stringify(lwtStyles));
+                                 JSON.stringify(lwtData));
   }
 }
 
 this.theme = class extends ExtensionAPI {
   onManifestEntry(entryName) {
     if (!gThemesEnabled) {
       // Return early if themes are disabled.
       return;
--- a/toolkit/components/extensions/test/browser/browser_ext_themes_lwtsupport.js
+++ b/toolkit/components/extensions/test/browser/browser_ext_themes_lwtsupport.js
@@ -65,52 +65,52 @@ add_task(async function test_LWT_image_a
   let docEl = window.document.documentElement;
   Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
   Assert.ok(!docEl.hasAttribute("lwtheme-image"), "LWT image attribute should not be set");
   await extension.unload();
   Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
   Assert.ok(!docEl.hasAttribute("lwtheme-image"), "LWT image attribute should not be set");
 });
 
-add_task(async function test_LWT_requires_accentcolor_defined_textcolor_only() {
+add_task(async function test_LWT_does_not_require_accentcolor_textcolor_only() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "theme": {
         "colors": {
           "textcolor": TEXT_COLOR,
         },
       },
     },
   });
 
   await extension.startup();
 
   let docEl = window.document.documentElement;
-  Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
+  Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
   Assert.ok(!docEl.hasAttribute("lwtheme-image"), "LWT image attribute should not be set");
   await extension.unload();
   Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
   Assert.ok(!docEl.hasAttribute("lwtheme-image"), "LWT image attribute should not be set");
 });
 
-add_task(async function test_LWT_requires_accentcolor_defined_image_only() {
+add_task(async function test_LWT_does_not_require_accentcolor_image_only() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "theme": {
         "images": {
           "headerURL": "image1.png",
         },
       },
     },
     files: {
       "image1.png": BACKGROUND,
     },
   });
 
   await extension.startup();
 
   let docEl = window.document.documentElement;
-  Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
-  Assert.ok(!docEl.hasAttribute("lwtheme-image"), "LWT image attribute should not be set");
+  Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
+  Assert.ok(docEl.hasAttribute("lwtheme-image"), "LWT image attribute should be set");
   await extension.unload();
   Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
   Assert.ok(!docEl.hasAttribute("lwtheme-image"), "LWT image attribute should not be set");
 });
--- a/toolkit/modules/LightweightThemeConsumer.jsm
+++ b/toolkit/modules/LightweightThemeConsumer.jsm
@@ -3,16 +3,19 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 var EXPORTED_SYMBOLS = ["LightweightThemeConsumer"];
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 
+const DEFAULT_THEME_ID = "default-theme@mozilla.org";
+const ICONS = Services.prefs.getStringPref("extensions.webextensions.themes.icons.buttons", "").split(",");
+
 const toolkitVariableMap = [
   ["--lwt-accent-color", {
     lwtProperty: "accentcolor",
     processColor(rgbaChannels, element) {
       if (!rgbaChannels || rgbaChannels.a == 0) {
         return "white";
       }
       // Remove the alpha channel
@@ -134,22 +137,26 @@ LightweightThemeConsumer.prototype = {
   observe(aSubject, aTopic, aData) {
     if (aTopic != "lightweight-theme-styling-update")
       return;
 
     const { outerWindowID } = this._win
       .QueryInterface(Ci.nsIInterfaceRequestor)
       .getInterface(Ci.nsIDOMWindowUtils);
 
-    const parsedData = JSON.parse(aData);
-    if (parsedData && parsedData.window && parsedData.window !== outerWindowID) {
+    let parsedData = JSON.parse(aData);
+    if (!parsedData) {
+      parsedData = { theme: null };
+    }
+
+    if (parsedData.window && parsedData.window !== outerWindowID) {
       return;
     }
 
-    this._update(parsedData);
+    this._update(parsedData.theme);
   },
 
   receiveMessage({ name, target }) {
     if (name == "LightweightTheme:Request") {
       let contentThemeData = _getContentProperties(this._doc, this._active, this._lastData);
       target.messageManager.sendAsyncMessage("LightweightTheme:Update", contentThemeData);
     }
   },
@@ -170,45 +177,47 @@ LightweightThemeConsumer.prototype = {
       case "EndSwapDocShells":
         let contentThemeData = _getContentProperties(this._doc, this._active, this._lastData);
         aEvent.target.messageManager.sendAsyncMessage("LightweightTheme:Update", contentThemeData);
         break;
     }
   },
 
   _update(aData) {
+    this._lastData = aData;
+    if (aData) {
+      aData = LightweightThemeImageOptimizer.optimize(aData, this._win.screen);
+    }
+
+    let active = this._active = !!aData && aData.id !== DEFAULT_THEME_ID;
+
     if (!aData) {
-      aData = { headerURL: "", footerURL: "", textcolor: "", accentcolor: "" };
-      this._lastData = aData;
-    } else {
-      this._lastData = aData;
-      aData = LightweightThemeImageOptimizer.optimize(aData, this._win.screen);
+      aData = {};
     }
 
     let root = this._doc.documentElement;
 
-    if (aData.headerURL) {
+    if (active && aData.headerURL) {
       root.setAttribute("lwtheme-image", "true");
     } else {
       root.removeAttribute("lwtheme-image");
     }
 
-    let active = aData.accentcolor || aData.headerURL;
-    this._active = active;
-
-    if (aData.icons) {
-      let activeIcons = active ? Object.keys(aData.icons).join(" ") : "";
+    if (active && aData.icons) {
+      let activeIcons = Object.keys(aData.icons).join(" ");
       root.setAttribute("lwthemeicons", activeIcons);
-      for (let [name, value] of Object.entries(aData.icons)) {
-        _setImage(root, active, name, value);
-      }
     } else {
       root.removeAttribute("lwthemeicons");
     }
 
+    for (let icon of ICONS) {
+      let value = aData.icons ? aData.icons[`--${icon}-icon`] : null;
+      _setImage(root, active, `--${icon}-icon`, value);
+    }
+
     _setImage(root, active, "--lwt-header-image", aData.headerURL);
     _setImage(root, active, "--lwt-footer-image", aData.footerURL);
     _setImage(root, active, "--lwt-additional-images", aData.additionalBackgrounds);
     _setProperties(root, active, aData);
 
     if (active) {
       root.setAttribute("lwtheme", "true");
     } else {
--- a/toolkit/mozapps/extensions/LightweightThemeManager.jsm
+++ b/toolkit/mozapps/extensions/LightweightThemeManager.jsm
@@ -894,17 +894,17 @@ function _updateUsedThemes(aList) {
 
   _prefs.setStringPref("usedThemes", JSON.stringify(aList));
 
   Services.obs.notifyObservers(null, "lightweight-theme-list-changed");
 }
 
 function _notifyWindows(aThemeData) {
   Services.obs.notifyObservers(null, "lightweight-theme-styling-update",
-                               JSON.stringify(aThemeData));
+                               JSON.stringify({theme: aThemeData}));
 }
 
 var _previewTimer;
 var _previewTimerCallback = {
   notify() {
     LightweightThemeManager.resetPreview();
   }
 };