Bug 1435117 - Use a different name for the icon colors internally since the theme object gets flattened when used by LightweightThemeConsumer. r=ntim
authorJared Wein <jwein@mozilla.com>
Thu, 01 Feb 2018 22:07:58 -0500
changeset 457204 76177a3d92b0c34bbd1d61a4d0b1271f3018d392
parent 457203 6c8eb870b45f80661450511f45d2c14d3d1b2aec
child 457205 6b68b2658dc4328054e7b27f97451a504ccebab0
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersntim
bugs1435117
milestone60.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 1435117 - Use a different name for the icon colors internally since the theme object gets flattened when used by LightweightThemeConsumer. r=ntim The 'icons' object already exists for a prototype way to allow theme authors to change the icons and it was conflicting with the newly introduced colors.icons pproperty that was added recently. MozReview-Commit-ID: Hwn9fYN1kcC
toolkit/components/extensions/ext-theme.js
toolkit/components/extensions/test/browser/browser_ext_themes_toolbarbutton_icons.js
toolkit/modules/LightweightThemeConsumer.jsm
--- a/toolkit/components/extensions/ext-theme.js
+++ b/toolkit/components/extensions/ext-theme.js
@@ -138,25 +138,29 @@ class Theme {
           break;
         case "toolbar":
           this.lwtStyles.toolbarColor = cssColor;
           break;
         case "toolbar_text":
         case "bookmark_text":
           this.lwtStyles.toolbar_text = cssColor;
           break;
+        case "icons":
+          this.lwtStyles.icon_color = cssColor;
+          break;
+        case "icons_attention":
+          this.lwtStyles.icon_attention_color = cssColor;
+          break;
         case "tab_text":
         case "toolbar_field":
         case "toolbar_field_text":
         case "toolbar_field_border":
         case "toolbar_top_separator":
         case "toolbar_bottom_separator":
         case "toolbar_vertical_separator":
-        case "icons":
-        case "icons_attention":
           this.lwtStyles[color] = cssColor;
           break;
       }
     }
   }
 
   /**
    * Helper method for loading images found in the extension's manifest.
--- a/toolkit/components/extensions/test/browser/browser_ext_themes_toolbarbutton_icons.js
+++ b/toolkit/components/extensions/test/browser/browser_ext_themes_toolbarbutton_icons.js
@@ -1,14 +1,14 @@
 "use strict";
 
 // This test checks applied WebExtension themes that attempt to change
 // icon color properties
 
-add_task(async function test_tint_properties() {
+add_task(async function test_icons_properties() {
   const ICONS_COLOR = "#001b47";
   const ICONS_ATTENTION_COLOR = "#44ba77";
 
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "theme": {
         "images": {
           "headerURL": "image1.png",
@@ -49,8 +49,56 @@ add_task(async function test_tint_proper
     `rgb(${hexToRGB(ICONS_ATTENTION_COLOR).join(", ")})`,
     "Starred icon fill is properly set"
   );
 
   starButton.removeAttribute("starred");
 
   await extension.unload();
 });
+
+add_task(async function test_no_icons_properties() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "theme": {
+        "images": {
+          "headerURL": "image1.png",
+        },
+        "colors": {
+          "accentcolor": ACCENT_COLOR,
+          "textcolor": TEXT_COLOR,
+        },
+      },
+    },
+    files: {
+      "image1.png": BACKGROUND,
+    },
+  });
+
+  await extension.startup();
+
+  let toolbarbutton = document.querySelector("#home-button");
+  let toolbarbuttonCS = window.getComputedStyle(toolbarbutton);
+  Assert.equal(
+    toolbarbuttonCS.getPropertyValue("--lwt-toolbarbutton-icon-fill"),
+    "",
+    "Icon fill should not be set when the value is not specified in the manifest."
+  );
+  let currentColor = toolbarbuttonCS.getPropertyValue("color");
+  Assert.equal(
+    window.getComputedStyle(toolbarbutton).getPropertyValue("fill"),
+    currentColor,
+    "Button fill color should be currentColor when no icon color specified."
+  );
+
+  let starButton = document.querySelector("#star-button");
+  starButton.setAttribute("starred", "true");
+  let starComputedStyle = window.getComputedStyle(starButton);
+  Assert.equal(
+    starComputedStyle.getPropertyValue("--lwt-toolbarbutton-icon-fill-attention"),
+    "",
+    "Icon attention fill should not be set when the value is not specified in the manifest."
+  );
+  starButton.removeAttribute("starred");
+
+  await extension.unload();
+});
+
--- a/toolkit/modules/LightweightThemeConsumer.jsm
+++ b/toolkit/modules/LightweightThemeConsumer.jsm
@@ -21,18 +21,18 @@ const kCSSVarsMap = new Map([
   ["--toolbar-bgcolor", "toolbarColor"],
   ["--toolbar-color", "toolbar_text"],
   ["--url-and-searchbar-background-color", "toolbar_field"],
   ["--url-and-searchbar-color", "toolbar_field_text"],
   ["--lwt-toolbar-field-border-color", "toolbar_field_border"],
   ["--tabs-border-color", "toolbar_top_separator"],
   ["--toolbox-border-bottom-color", "toolbar_bottom_separator"],
   ["--urlbar-separator-color", "toolbar_vertical_separator"],
-  ["--lwt-toolbarbutton-icon-fill", "icons"],
-  ["--lwt-toolbarbutton-icon-fill-attention", "icons_attention"],
+  ["--lwt-toolbarbutton-icon-fill", "icon_color"],
+  ["--lwt-toolbarbutton-icon-fill-attention", "icon_attention_color"],
 ]);
 
 this.LightweightThemeConsumer =
  function LightweightThemeConsumer(aDocument) {
   this._doc = aDocument;
   this._win = aDocument.defaultView;
 
   let screen = this._win.screen;