Bug 1550090 - Fix theme active check. r=dao
☠☠ backed out by 53380c05b1dc ☠ ☠
authorTim Nguyen <ntim.bugs@gmail.com>
Sat, 11 May 2019 09:32:49 +0000
changeset 532331 58acbc1673310d89dda93756db7cb6d6abf8e30e
parent 532330 dba3c6692a02d1e874bbdbfae9a8424bb68375d3
child 532332 c0a837d2a3778981498a2c6a1afd35eb86f0dc95
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao
bugs1550090
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1550090 - Fix theme active check. r=dao Differential Revision: https://phabricator.services.mozilla.com/D30396
toolkit/components/extensions/parent/ext-theme.js
toolkit/modules/LightweightThemeConsumer.jsm
toolkit/mozapps/extensions/test/browser/browser_webapi_theme.js
--- a/toolkit/components/extensions/parent/ext-theme.js
+++ b/toolkit/components/extensions/parent/ext-theme.js
@@ -40,30 +40,21 @@ class Theme {
     this.extension = extension;
     this.details = details;
     this.darkDetails = darkDetails;
     this.windowId = windowId;
 
     if (startupData && startupData.lwtData) {
       Object.assign(this, startupData);
     } else {
-      // TODO(ntim): clean this in bug 1550090
       this.lwtStyles = {};
-      this.lwtDarkStyles = null;
-      if (darkDetails) {
-        this.lwtDarkStyles = {};
-      }
+      this.lwtDarkStyles = {};
 
       if (experiment) {
         if (extension.experimentsAllowed) {
-          this.lwtStyles.experimental = {
-            colors: {},
-            images: {},
-            properties: {},
-          };
           const {baseURI} = this.extension;
           if (experiment.stylesheet) {
             experiment.stylesheet = baseURI.resolve(experiment.stylesheet);
           }
           this.experiment = experiment;
         } else {
           const {logger} = this.extension;
           logger.warn("This extension is not allowed to run theme experiments");
@@ -86,16 +77,18 @@ class Theme {
       this.loadDetails(this.details, this.lwtStyles);
       if (this.darkDetails) {
         this.loadDetails(this.darkDetails, this.lwtDarkStyles);
       }
 
       this.lwtData = {
         theme: this.lwtStyles,
         darkTheme: this.lwtDarkStyles,
+        id: this.extension.id,
+        version: this.extension.version,
       };
 
       if (this.experiment) {
         this.lwtData.experiment = this.experiment;
       }
 
       this.extension.startupData = {
         lwtData: this.lwtData,
@@ -121,29 +114,35 @@ class Theme {
                                  "lightweight-theme-styling-update");
   }
 
   /**
    * @param {Object} details Details
    * @param {Object} styles Styles object in which to store the colors.
    */
   loadDetails(details, styles) {
+    if (this.experiment) {
+      styles.experimental = {
+        colors: {},
+        images: {},
+        properties: {},
+      };
+    }
+
     if (details.colors) {
       this.loadColors(details.colors, styles);
     }
 
     if (details.images) {
       this.loadImages(details.images, styles);
     }
 
     if (details.properties) {
       this.loadProperties(details.properties, styles);
     }
-
-    this.loadMetadata(this.extension, styles);
   }
 
   /**
    * Helper method for loading colors found in the extension's manifest.
    *
    * @param {Object} colors Dictionary mapping color properties to values.
    * @param {Object} styles Styles object in which to store the colors.
    */
@@ -332,31 +331,20 @@ class Theme {
             logger.warn(`Unrecognized theme property found: properties.${property}`);
           }
           break;
         }
       }
     }
   }
 
-  /**
-   * Helper method for loading extension metadata required by downstream
-   * consumers.
-   *
-   * @param {Object} extension Extension object.
-   * @param {Object} styles Styles object in which to store the colors.
-   */
-  loadMetadata(extension, styles) {
-    styles.id = extension.id;
-    styles.version = extension.version;
-  }
-
   static unload(windowId) {
     let lwtData = {
-      theme: null,
+      theme: {},
+      darkTheme: {},
     };
 
     if (windowId) {
       lwtData.window = getWinUtils(windowTracker.getWindow(windowId)).outerWindowID;
       windowOverrides.set(windowId, emptyTheme);
     } else {
       windowOverrides.clear();
       defaultTheme = emptyTheme;
--- a/toolkit/modules/LightweightThemeConsumer.jsm
+++ b/toolkit/modules/LightweightThemeConsumer.jsm
@@ -184,53 +184,49 @@ LightweightThemeConsumer.prototype = {
   get darkMode() {
     return this.darkThemeMediaQuery && this.darkThemeMediaQuery.matches;
   },
 
   _update(themeData) {
     this._lastData = themeData;
 
     let theme = themeData.theme;
-    if (themeData.darkTheme && this.darkMode) {
+    if (Object.keys(themeData.darkTheme).length && this.darkMode) {
       theme = themeData.darkTheme;
     }
-    if (!theme) {
-      theme = { id: DEFAULT_THEME_ID };
-    }
 
     let active = this._active = Object.keys(theme).length;
 
     let root = this._doc.documentElement;
 
     if (active && theme.headerURL) {
       root.setAttribute("lwtheme-image", "true");
     } else {
       root.removeAttribute("lwtheme-image");
     }
 
-    this._setExperiment(active, themeData.experiment, theme.experimental);
-
     if (theme.headerImage) {
       this._doc.mozSetImageElement("lwt-header-image", theme.headerImage);
       root.style.setProperty("--lwt-header-image", "-moz-element(#lwt-header-image)");
     } else {
       this._doc.mozSetImageElement("lwt-header-image", null);
       root.style.removeProperty("--lwt-header-image");
     }
 
     _setImage(root, active, "--lwt-additional-images", theme.additionalBackgrounds);
     _setProperties(root, active, theme);
+    this._setExperiment(active, themeData.experiment, theme.experimental);
 
-    if (theme.id != DEFAULT_THEME_ID || this.darkMode) {
+    if (active) {
       root.setAttribute("lwtheme", "true");
     } else {
       root.removeAttribute("lwtheme");
       root.removeAttribute("lwthemetextcolor");
     }
-    if (theme.id == DEFAULT_THEME_ID && this.darkMode) {
+    if (themeData.id == DEFAULT_THEME_ID && this.darkMode) {
       root.setAttribute("lwt-default-theme-in-dark-mode", "true");
     } else {
       root.removeAttribute("lwt-default-theme-in-dark-mode");
     }
 
     let contentThemeData = _getContentProperties(this._doc, active, theme);
     Services.ppmm.sharedData.set(`theme/${this._winId}`, contentThemeData);
   },
--- a/toolkit/mozapps/extensions/test/browser/browser_webapi_theme.js
+++ b/toolkit/mozapps/extensions/test/browser/browser_webapi_theme.js
@@ -44,15 +44,15 @@ add_task(async function test_theme_insta
     ok(true, "Theme install completed");
 
     await BrowserTestUtils.closeWindow(newWin);
 
     Assert.equal(updates.length, 1, "Got a single theme update");
     let parsed = JSON.parse(updates[0]);
     ok(parsed.theme.headerURL.endsWith("/testImage.png"),
        "Theme update has the expected headerURL");
-    is(parsed.theme.id, "theme@tests.mozilla.org", "Theme update includes the theme ID");
-    is(parsed.theme.version, "1.0", "Theme update includes the theme's version");
+    is(parsed.id, "theme@tests.mozilla.org", "Theme update includes the theme ID");
+    is(parsed.version, "1.0", "Theme update includes the theme's version");
 
-    let addon = await AddonManager.getAddonByID(parsed.theme.id);
+    let addon = await AddonManager.getAddonByID(parsed.id);
     await addon.uninstall();
   });
 });