Bug 1449327 - Stop sanitizing fully transparent values. r=dao
authorTim Nguyen <ntim.bugs@gmail.com>
Wed, 11 Apr 2018 21:05:13 +0200
changeset 412890 2865f00b0d4357580a59d955fddacc0140876410
parent 412889 cfc40d3357e1e8779ec1ee4554d0fae08bf7cc86
child 412891 32ae04d4e132d6d610be4a41df9801bcd1f4df58
push id33823
push userebalazs@mozilla.com
push dateThu, 12 Apr 2018 09:38:35 +0000
treeherdermozilla-central@abd91e812e7e [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: DRQjCm35QsN
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", {
@@ -199,32 +199,33 @@ function _setProperties(root, active, th
     }
   }
 }
 
 function _sanitizeCSSColor(doc, cssColor) {
   if (!cssColor) {
     return null;
   }
-  // style.color normalizes color values and rejects invalid ones, so a
+  const HTML_NS = "http://www.w3.org/1999/xhtml";
+  // style.color normalizes color values and makes invalid ones black, so a
   // simple round trip gets us a sanitized color value.
-  let span = doc.createElementNS("http://www.w3.org/1999/xhtml", "span");
+  let div = doc.createElementNS(HTML_NS, "div");
+  div.style.color = "black";
+  let span = doc.createElementNS(HTML_NS, "span");
   span.style.color = cssColor;
+  div.appendChild(span);
   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,
   };
 }