Bug 1449327 - Stop sanitizing fully transparent values. r=dao
☠☠ backed out by 60de60fe2733 ☠ ☠
authorTim Nguyen <ntim.bugs@gmail.com>
Sat, 31 Mar 2018 13:57:09 +0200
changeset 468703 9d0fc1d12d9a29d989df38bf74747490a86b114f
parent 468702 3542146e997310c11ab000341cce6360623fce30
child 468704 d34178082ca879e801132c333de935de37280e1e
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
bugs1449327
milestone61.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 1449327 - Stop sanitizing fully transparent values. r=dao MozReview-Commit-ID: JMtBYycoRMC
toolkit/components/extensions/test/browser/browser.ini
toolkit/components/extensions/test/browser/browser_ext_themes_sanitization.js
toolkit/modules/LightweightThemeConsumer.jsm
--- a/toolkit/components/extensions/test/browser/browser.ini
+++ b/toolkit/components/extensions/test/browser/browser.ini
@@ -20,9 +20,10 @@ support-files =
 [browser_ext_themes_toolbar_fields_focus.js]
 [browser_ext_themes_toolbar_fields.js]
 [browser_ext_themes_toolbars.js]
 [browser_ext_themes_toolbarbutton_icons.js]
 [browser_ext_themes_toolbarbutton_colors.js]
 [browser_ext_themes_theme_transition.js]
 [browser_ext_themes_arrowpanels.js]
 [browser_ext_themes_tab_selected.js]
-[browser_ext_themes_autocomplete_popup.js]
\ No newline at end of file
+[browser_ext_themes_autocomplete_popup.js]
+[browser_ext_themes_sanitization.js]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/browser/browser_ext_themes_sanitization.js
@@ -0,0 +1,128 @@
+"use strict";
+
+// This test checks color sanitization in various situations
+
+add_task(async function test_sanitization_invalid() {
+  // This test checks that invalid values are sanitized
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "theme": {
+        "colors": {
+          "accentcolor": ACCENT_COLOR,
+          "textcolor": TEXT_COLOR,
+          "toolbar_bottom_separator": "ntimsfavoriteblue",
+        },
+      },
+    },
+  });
+
+  let docEl = document.documentElement;
+
+  let transitionPromise = waitForTransition(docEl, "background-color");
+  await extension.startup();
+  await transitionPromise;
+
+  let toolbox = document.querySelector("#navigator-toolbox");
+  Assert.equal(
+    window.getComputedStyle(toolbox, "::after").borderBottomColor,
+    "rgb(0, 0, 0)",
+    "All invalid values should always compute to black."
+  );
+
+  await extension.unload();
+});
+
+add_task(async function test_sanitization_css_variables() {
+  // This test checks that CSS variables are sanitized
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "theme": {
+        "colors": {
+          "accentcolor": ACCENT_COLOR,
+          "textcolor": TEXT_COLOR,
+          "toolbar_bottom_separator": "var(--arrowpanel-dimmed)",
+        },
+      },
+    },
+  });
+
+  let docEl = document.documentElement;
+
+  let transitionPromise = waitForTransition(docEl, "background-color");
+  await extension.startup();
+  await transitionPromise;
+
+  let toolbox = document.querySelector("#navigator-toolbox");
+  Assert.equal(
+    window.getComputedStyle(toolbox, "::after").borderBottomColor,
+    "rgb(0, 0, 0)",
+    "All CSS variables should always compute to black."
+  );
+
+  await extension.unload();
+});
+
+add_task(async function test_sanitization_transparent() {
+  // This test checks whether transparent values are applied properly
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "theme": {
+        "colors": {
+          "accentcolor": ACCENT_COLOR,
+          "textcolor": TEXT_COLOR,
+          "toolbar_top_separator": "transparent",
+          "toolbar_bottom_separator": "transparent",
+        },
+      },
+    },
+  });
+
+  let docEl = document.documentElement;
+
+  let transitionPromise = waitForTransition(docEl, "background-color");
+  await extension.startup();
+  await transitionPromise;
+
+  let navbar = document.querySelector("#nav-bar");
+  Assert.ok(
+    window.getComputedStyle(navbar).boxShadow.includes("rgba(0, 0, 0, 0)"),
+    "Top separator should be transparent"
+  );
+
+  let toolbox = document.querySelector("#navigator-toolbox");
+  Assert.equal(
+    window.getComputedStyle(toolbox, "::after").borderBottomColor,
+    "rgba(0, 0, 0, 0)",
+    "Bottom separator should be transparent"
+  );
+
+  await extension.unload();
+});
+
+add_task(async function test_sanitization_transparent_accentcolor() {
+  // This test checks whether transparent accentcolor falls back to white.
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "theme": {
+        "colors": {
+          "accentcolor": "transparent",
+          "textcolor": TEXT_COLOR,
+        },
+      },
+    },
+  });
+
+  let docEl = document.documentElement;
+
+  let transitionPromise = waitForTransition(docEl, "background-color");
+  await extension.startup();
+  await transitionPromise;
+
+  Assert.equal(
+    window.getComputedStyle(docEl).backgroundColor,
+    "rgb(255, 255, 255)",
+    "Accent color should be white",
+  );
+
+  await extension.unload();
+});
--- a/toolkit/modules/LightweightThemeConsumer.jsm
+++ b/toolkit/modules/LightweightThemeConsumer.jsm
@@ -7,17 +7,17 @@ var EXPORTED_SYMBOLS = ["LightweightThem
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 
 const toolkitVariableMap = [
   ["--lwt-accent-color", {
     lwtProperty: "accentcolor",
     processColor(rgbaChannels, element) {
-      if (!rgbaChannels) {
+      if (!rgbaChannels || rgbaChannels.a == 0) {
         return "white";
       }
       // Remove the alpha channel
       const {r, g, b} = rgbaChannels;
       return `rgb(${r}, ${g}, ${b})`;
     }
   }],
   ["--lwt-text-color", {
@@ -204,27 +204,24 @@ function _sanitizeCSSColor(doc, cssColor
   if (!cssColor) {
     return null;
   }
   // style.color normalizes color values and rejects invalid ones, so a
   // simple round trip gets us a sanitized color value.
   let span = doc.createElementNS("http://www.w3.org/1999/xhtml", "span");
   span.style.color = cssColor;
   cssColor = doc.defaultView.getComputedStyle(span).color;
-  if (cssColor == "rgba(0, 0, 0, 0)") {
-    return null;
-  }
   return cssColor;
 }
 
 function _parseRGBA(aColorString) {
   if (!aColorString) {
     return null;
   }
   var rgba = aColorString.replace(/(rgba?\()|(\)$)/g, "").split(",");
   rgba = rgba.map(x => parseFloat(x));
   return {
     r: rgba[0],
     g: rgba[1],
     b: rgba[2],
-    a: rgba[3] || 1,
+    a: 3 in rgba ? rgba[3] : 1,
   };
 }