Bug 1460287 - Use black as a fallback for textcolor and always set lwtheme attribute if theme is active. r=dao, a=RyanVM
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 11 May 2018 18:48:53 +0100
changeset 473340 305d61ed533a4e3672d278128322f564dde378d6
parent 473339 6247c3de81214476abc9d6f30465db4a4402441f
child 473341 594d4ff3b55bca69fedc366b0573bf17e204f118
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao, RyanVM
bugs1460287
milestone61.0
Bug 1460287 - Use black as a fallback for textcolor and always set lwtheme attribute if theme is active. r=dao, a=RyanVM MozReview-Commit-ID: 30NKcuz94d
toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js
toolkit/components/extensions/test/browser/browser_ext_themes_sanitization.js
toolkit/modules/LightweightThemeConsumer.jsm
--- a/toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js
+++ b/toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js
@@ -3,17 +3,17 @@
 add_task(async function setup() {
   await SpecialPowers.pushPrefEnv({
     set: [["extensions.webextensions.themes.enabled", true]],
   });
 });
 
 add_task(async function test_support_theme_frame() {
   const FRAME_COLOR = [71, 105, 91];
-  const TAB_TEXT_COLOR = [207, 221, 192, .9];
+  const TAB_TEXT_COLOR = [207, 221, 192];
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "theme": {
         "images": {
           "theme_frame": "face.png",
         },
         "colors": {
           "frame": FRAME_COLOR,
@@ -35,28 +35,28 @@ add_task(async function test_support_the
   Assert.equal(docEl.getAttribute("lwthemetextcolor"), "bright",
                "LWT text color attribute should be set");
 
   let style = window.getComputedStyle(docEl);
   Assert.ok(style.backgroundImage.includes("face.png"),
             `The backgroundImage should use face.png. Actual value is: ${style.backgroundImage}`);
   Assert.equal(style.backgroundColor, "rgb(" + FRAME_COLOR.join(", ") + ")",
                "Expected correct background color");
-  Assert.equal(style.color, "rgba(" + TAB_TEXT_COLOR.join(", ") + ")",
+  Assert.equal(style.color, "rgb(" + TAB_TEXT_COLOR.join(", ") + ")",
                "Expected correct text color");
 
   await extension.unload();
 
   Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
 });
 
 add_task(async function test_support_theme_frame_inactive() {
   const FRAME_COLOR = [71, 105, 91];
   const FRAME_COLOR_INACTIVE = [255, 0, 0];
-  const TAB_TEXT_COLOR = [207, 221, 192, .9];
+  const TAB_TEXT_COLOR = [207, 221, 192];
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "theme": {
         "images": {
           "theme_frame": "image1.png",
         },
         "colors": {
           "frame": FRAME_COLOR,
@@ -91,17 +91,17 @@ add_task(async function test_support_the
   await BrowserTestUtils.closeWindow(window2);
   await extension.unload();
 
   Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
 });
 
 add_task(async function test_lack_of_theme_frame_inactive() {
   const FRAME_COLOR = [71, 105, 91];
-  const TAB_TEXT_COLOR = [207, 221, 192, .9];
+  const TAB_TEXT_COLOR = [207, 221, 192];
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "theme": {
         "images": {
           "theme_frame": "image1.png",
         },
         "colors": {
           "frame": FRAME_COLOR,
--- a/toolkit/components/extensions/test/browser/browser_ext_themes_sanitization.js
+++ b/toolkit/components/extensions/test/browser/browser_ext_themes_sanitization.js
@@ -121,8 +121,36 @@ add_task(async function test_sanitizatio
   Assert.equal(
     window.getComputedStyle(docEl).backgroundColor,
     "rgb(255, 255, 255)",
     "Accent color should be white",
   );
 
   await extension.unload();
 });
+
+add_task(async function test_sanitization_transparent_textcolor() {
+  // This test checks whether transparent textcolor falls back to black.
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "theme": {
+        "colors": {
+          "accentcolor": ACCENT_COLOR,
+          "textcolor": "transparent",
+        },
+      },
+    },
+  });
+
+  let docEl = document.documentElement;
+
+  let transitionPromise = waitForTransition(docEl, "background-color");
+  await extension.startup();
+  await transitionPromise;
+
+  Assert.equal(
+    window.getComputedStyle(docEl).color,
+    "rgb(0, 0, 0)",
+    "Text color should be black",
+  );
+
+  await extension.unload();
+});
--- a/toolkit/modules/LightweightThemeConsumer.jsm
+++ b/toolkit/modules/LightweightThemeConsumer.jsm
@@ -19,25 +19,23 @@ const toolkitVariableMap = [
       const {r, g, b} = rgbaChannels;
       return `rgb(${r}, ${g}, ${b})`;
     }
   }],
   ["--lwt-text-color", {
     lwtProperty: "textcolor",
     processColor(rgbaChannels, element) {
       if (!rgbaChannels) {
-        element.removeAttribute("lwthemetextcolor");
-        element.removeAttribute("lwtheme");
-        return null;
+        rgbaChannels = {r: 0, g: 0, b: 0};
       }
-      const {r, g, b, a} = rgbaChannels;
+      // Remove the alpha channel
+      const {r, g, b} = rgbaChannels;
       const luminance = _getLuminance(r, g, b);
       element.setAttribute("lwthemetextcolor", luminance <= 110 ? "dark" : "bright");
-      element.setAttribute("lwtheme", "true");
-      return `rgba(${r}, ${g}, ${b}, ${a})` || "black";
+      return `rgba(${r}, ${g}, ${b})`;
     }
   }],
   ["--arrowpanel-background", {
     lwtProperty: "popup"
   }],
   ["--arrowpanel-color", {
     lwtProperty: "popup_text"
   }],
@@ -174,16 +172,23 @@ LightweightThemeConsumer.prototype = {
       root.removeAttribute("lwthemeicons");
     }
 
     _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 {
+      root.removeAttribute("lwtheme");
+      root.removeAttribute("lwthemetextcolor");
+    }
+
     if (active && aData.footerURL)
       root.setAttribute("lwthemefooter", "true");
     else
       root.removeAttribute("lwthemefooter");
 
     Services.obs.notifyObservers(this._win, "lightweight-theme-window-updated",
                                  JSON.stringify(aData));
   }