Bug 1594132 - Move background-color rules so they do not appear under appearance rules, enabling sidebar vibrancy. r=ntim,dao

`appearance` CSS rules allow elements to take on system appearance. For UI elements that we want to take on system styling, we set `appearance: auto` combined with platform-specific rules like `-moz-default-appearance: -moz-mac-vibrant-titlebar-light;`

macOS sidebar vibrancy broke because a background-color was being applied to `root`. That colour appeared under elements with `appearance: auto` set, so we wouldn't see the platform-specific styling. This patch moves the root background-color to `#navigator-toolbox`, so that it does not appear under `#sidebar-box`.

We still want a background colour applied to sidebars when a lwtheme includes one. We only want `appearance: auto` applied to sidebars when the active theme does not have sidebar styling rules. That's why `#sidebar-box:not(:-moz-lwtheme)` is changed to `#sidebar-box:not([lwt-sidebar])`.

This patch also removes the rule
```
:root:-moz-lwtheme {
  appearance: none;
}
```
from osx/global/global.css. There's no corresponding addition of a `#navigator-toolbox { appearance:none; }` rule because that rule already exists in [osx/browser.css](https://searchfox.org/mozilla-central/rev/7067bbd8194f4346ec59d77c33cd88f06763e090/browser/themes/osx/browser.css#45).

Differential Revision: https://phabricator.services.mozilla.com/D104416

browser/base/content/browser.css

@@ -38,20 +38,9 @@ body {
   --toolbar-bgimage: none;
   --toolbar-color: var(--lwt-text-color, inherit);
 
-  background-color: var(--lwt-accent-color);
   color: var(--lwt-text-color);
 }
 
-:root:-moz-lwtheme[lwtheme-image] {
-  background-image: var(--lwt-header-image) !important;
-  background-repeat: no-repeat;
-  background-position: right top !important;
-}
-
-:root:-moz-lwtheme:-moz-window-inactive {
-  background-color: var(--lwt-accent-color-inactive, var(--lwt-accent-color));
-}
-
 :root:not([chromehidden~="toolbar"]) {
   min-width: 450px;
 }
@@ -72,12 +61,23 @@ body {
   -moz-box-flex: 1;
 }
 
-/* Set additional backgrounds alignment relative to toolbox */
-
 #navigator-toolbox:-moz-lwtheme {
+  background-color: var(--lwt-accent-color);
   background-image: var(--lwt-additional-images);
-  background-position: var(--lwt-background-alignment);
   background-repeat: var(--lwt-background-tiling);
+  background-position: var(--lwt-background-alignment);
+}
+
+/* When a theme defines both theme_frame and additional_backgrounds, show
+   the latter atop the former. */
+:root[lwtheme-image] #navigator-toolbox {
+  background-image: var(--lwt-header-image), var(--lwt-additional-images);
+  background-repeat: no-repeat, var(--lwt-background-tiling);
+  background-position: right top, var(--lwt-background-alignment);
+}
+
+#navigator-toolbox:-moz-window-inactive:-moz-lwtheme {
+  background-color: var(--lwt-accent-color-inactive, var(--lwt-accent-color));
 }
 
 .search-one-offs[compact=true] .search-setting-button,

browser/components/extensions/test/browser/browser_ext_management.js

@@ -112,7 +112,9 @@ add_task(async function test_management_install() {
   is(id, "tiger@persona.beard", "Static web extension theme installed");
   is(type, "theme", "Extension type is correct");
 
-  let style = window.getComputedStyle(document.documentElement);
+  let style = window.getComputedStyle(
+    document.documentElement.querySelector("#navigator-toolbox")
+  );
   is(style.backgroundColor, "rgb(255, 165, 0)", "Background is the new black");
 
   let addon = await AddonManager.getAddonByID("tiger@persona.beard");

browser/themes/osx/browser.css

@@ -453,13 +453,11 @@
   --sidebar-background-color: -moz-mac-source-list;
 }
 
-/* Give the sidebar a vibrant appearance. Only do this when no lwtheme is
- * in use, because vibrant appearance values only work if there is no
- * background-color rendered behind the element, and we have :root:-moz-lwtheme
- * rules which set appearance: none and an opaque background color on the
- * root element. That background color would interfere with the vibrancy.
- * See bug 1594132 for fixing this. */
-#sidebar-box:not(:-moz-lwtheme) {
+/* Give the sidebar a vibrant appearance. Only do this when no lwtheme sidebar
+ * rules are in use. Vibrant appearance values only work if there is no
+ * background-color rendered behind the element. If the active theme has sidebar
+ * rules, we want to show the theme's background-color in the sidebar. */
+#sidebar-box:not([lwt-sidebar]) {
   appearance: auto;
   -moz-default-appearance: -moz-mac-source-list;
   -moz-font-smoothing-background-color: -moz-mac-source-list;

browser/themes/osx/compacttheme.css

@@ -8,7 +8,7 @@
  * macOS vibrancy effects only work when there is no background colour behind
  * the effect.
  */
-:root:-moz-lwtheme-darktext {
+#navigator-toolbox:-moz-lwtheme-darktext {
   background-color: transparent;
 }
 

toolkit/components/extensions/test/browser/browser_ext_themes_additional_backgrounds_alignment.js

@@ -25,23 +25,19 @@ add_task(async function test_default_additional_backgrounds_alignment() {
 
   await extension.startup();
 
-  let docEl = document.documentElement;
-  let rootCS = window.getComputedStyle(docEl);
-
-  Assert.equal(
-    rootCS.getPropertyValue("background-position"),
-    RIGHT_TOP,
-    "root only contains theme_frame alignment property"
-  );
-
   let toolbox = document.querySelector("#navigator-toolbox");
   let toolboxCS = window.getComputedStyle(toolbox);
 
+  /**
+   * We expect duplicate background-position values because we apply `right top`
+   * once for theme_frame, and again as the default value of
+   * --lwt-background-alignment.
+   */
   Assert.equal(
     toolboxCS.getPropertyValue("background-position"),
-    RIGHT_TOP,
+    `${RIGHT_TOP}, ${RIGHT_TOP}`,
     toolbox.id +
-      " only contains default additional backgrounds alignment property"
+      " contains theme_frame and default lwt-background-alignment properties"
   );
 
   await extension.unload();
@@ -80,22 +76,14 @@ add_task(async function test_additional_backgrounds_alignment() {
 
   await extension.startup();
 
-  let docEl = document.documentElement;
-  let rootCS = window.getComputedStyle(docEl);
-
-  Assert.equal(
-    rootCS.getPropertyValue("background-position"),
-    RIGHT_TOP,
-    "root only contains theme_frame alignment property"
-  );
-
   let toolbox = document.querySelector("#navigator-toolbox");
   let toolboxCS = window.getComputedStyle(toolbox);
 
   Assert.equal(
     toolboxCS.getPropertyValue("background-position"),
-    LEFT_BOTTOM + ", " + CENTER_CENTER + ", " + RIGHT_TOP,
-    toolbox.id + " contains additional backgrounds alignment properties"
+    RIGHT_TOP + ", " + LEFT_BOTTOM + ", " + CENTER_CENTER + ", " + RIGHT_TOP,
+    toolbox.id +
+      " contains theme_frame and additional_backgrounds alignment properties"
   );
 
   await extension.unload();

toolkit/components/extensions/test/browser/browser_ext_themes_alpha_accentcolor.js

@@ -21,11 +21,11 @@ add_task(async function test_alpha_frame_color() {
   await extension.startup();
 
   // Add the event listener before loading the extension
-  let docEl = window.document.documentElement;
-  let style = window.getComputedStyle(docEl);
+  let toolbox = document.querySelector("#navigator-toolbox");
+  let toolboxCS = window.getComputedStyle(toolbox);
 
   Assert.equal(
-    style.backgroundColor,
+    toolboxCS.backgroundColor,
     "rgb(230, 128, 0)",
     "Window background color should be opaque"
   );

toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js

@@ -36,18 +36,20 @@ add_task(async function test_support_theme_frame() {
     "LWT text color attribute should be set"
   );
 
-  let style = window.getComputedStyle(docEl);
+  let toolbox = document.querySelector("#navigator-toolbox");
+  let toolboxCS = window.getComputedStyle(toolbox);
+
   Assert.ok(
-    style.backgroundImage.includes("face.png"),
-    `The backgroundImage should use face.png. Actual value is: ${style.backgroundImage}`
+    toolboxCS.backgroundImage.includes("face.png"),
+    `The backgroundImage should use face.png. Actual value is: ${toolboxCS.backgroundImage}`
   );
   Assert.equal(
-    style.backgroundColor,
+    toolboxCS.backgroundColor,
     "rgb(" + FRAME_COLOR.join(", ") + ")",
     "Expected correct background color"
   );
   Assert.equal(
-    style.color,
+    toolboxCS.color,
     "rgb(" + TAB_TEXT_COLOR.join(", ") + ")",
     "Expected correct text color"
   );
@@ -92,10 +94,11 @@ add_task(async function test_support_theme_frame_inactive() {
   await extension.startup();
 
   let docEl = window.document.documentElement;
-  let style = window.getComputedStyle(docEl);
+  let toolbox = document.querySelector("#navigator-toolbox");
+  let toolboxCS = window.getComputedStyle(toolbox);
 
   Assert.equal(
-    style.backgroundColor,
+    toolboxCS.backgroundColor,
     "rgb(" + FRAME_COLOR.join(", ") + ")",
     "Window background is set to the colors.frame property"
   );
@@ -103,7 +106,7 @@ add_task(async function test_support_theme_frame_inactive() {
   // Now we'll open a new window to see if the inactive browser accent color changed
   let window2 = await BrowserTestUtils.openNewBrowserWindow();
   Assert.equal(
-    style.backgroundColor,
+    toolboxCS.backgroundColor,
     "rgb(" + FRAME_COLOR_INACTIVE.join(", ") + ")",
     `Inactive window background color should be ${FRAME_COLOR_INACTIVE}`
   );
@@ -137,10 +140,11 @@ add_task(async function test_lack_of_theme_frame_inactive() {
   await extension.startup();
 
   let docEl = window.document.documentElement;
-  let style = window.getComputedStyle(docEl);
+  let toolbox = document.querySelector("#navigator-toolbox");
+  let toolboxCS = window.getComputedStyle(toolbox);
 
   Assert.equal(
-    style.backgroundColor,
+    toolboxCS.backgroundColor,
     "rgb(" + FRAME_COLOR.join(", ") + ")",
     "Window background is set to the colors.frame property"
   );
@@ -149,7 +153,7 @@ add_task(async function test_lack_of_theme_frame_inactive() {
   let window2 = await BrowserTestUtils.openNewBrowserWindow();
 
   Assert.equal(
-    style.backgroundColor,
+    toolboxCS.backgroundColor,
     "rgb(" + FRAME_COLOR.join(", ") + ")",
     "Inactive window background should not change if colors.frame_inactive isn't set"
   );

toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js

@@ -21,7 +21,10 @@ function hexToRGB(hex) {
 
 function validateTheme(backgroundImage, accentColor, textColor, isLWT) {
   let docEl = window.document.documentElement;
-  let style = window.getComputedStyle(docEl);
+  let rootCS = window.getComputedStyle(docEl);
+
+  let toolbox = document.querySelector("#navigator-toolbox");
+  let toolboxCS = window.getComputedStyle(toolbox);
 
   if (isLWT) {
     Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
@@ -33,7 +36,7 @@ function validateTheme(backgroundImage, accentColor, textColor, isLWT) {
   }
 
   Assert.ok(
-    style.backgroundImage.includes(backgroundImage),
+    toolboxCS.backgroundImage.includes(backgroundImage),
     "Expected correct background image"
   );
   if (accentColor.startsWith("#")) {
@@ -43,11 +46,11 @@ function validateTheme(backgroundImage, accentColor, textColor, isLWT) {
     textColor = hexToRGB(textColor);
   }
   Assert.equal(
-    style.backgroundColor,
+    toolboxCS.backgroundColor,
     accentColor,
     "Expected correct accent color"
   );
-  Assert.equal(style.color, textColor, "Expected correct text color");
+  Assert.equal(rootCS.color, textColor, "Expected correct text color");
 }
 
 add_task(async function test_dynamic_theme_updates() {
@@ -74,7 +77,10 @@ add_task(async function test_dynamic_theme_updates() {
     },
   });
 
-  let defaultStyle = window.getComputedStyle(window.document.documentElement);
+  let rootCS = window.getComputedStyle(window.document.documentElement);
+  let toolboxCS = window.getComputedStyle(
+    window.document.documentElement.querySelector("#navigator-toolbox")
+  );
   await extension.startup();
 
   extension.sendMessage("update-theme", {
@@ -111,7 +117,8 @@ add_task(async function test_dynamic_theme_updates() {
 
   await extension.awaitMessage("theme-reset");
 
-  let { backgroundImage, backgroundColor, color } = defaultStyle;
+  let { color } = rootCS;
+  let { backgroundImage, backgroundColor } = toolboxCS;
   validateTheme(backgroundImage, backgroundColor, color, false);
 
   await extension.unload();
@@ -140,7 +147,10 @@ add_task(async function test_dynamic_theme_updates_with_data_url() {
     },
   });
 
-  let defaultStyle = window.getComputedStyle(window.document.documentElement);
+  let rootCS = window.getComputedStyle(window.document.documentElement);
+  let toolboxCS = window.getComputedStyle(
+    window.document.documentElement.querySelector("#navigator-toolbox")
+  );
   await extension.startup();
 
   extension.sendMessage("update-theme", {
@@ -175,7 +185,8 @@ add_task(async function test_dynamic_theme_updates_with_data_url() {
 
   await extension.awaitMessage("theme-reset");
 
-  let { backgroundImage, backgroundColor, color } = defaultStyle;
+  let { color } = rootCS;
+  let { backgroundImage, backgroundColor } = toolboxCS;
   validateTheme(backgroundImage, backgroundColor, color, false);
 
   await extension.unload();

toolkit/components/extensions/test/browser/browser_ext_themes_lwtsupport.js

@@ -27,7 +27,10 @@ add_task(async function test_deprecated_LWT_properties_ignored() {
   await extension.startup();
 
   let docEl = window.document.documentElement;
-  let style = window.getComputedStyle(docEl);
+  let docStyle = window.getComputedStyle(docEl);
+  let navigatorStyle = window.getComputedStyle(
+    docEl.querySelector("#navigator-toolbox")
+  );
 
   Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
   Assert.ok(
@@ -41,12 +44,12 @@ add_task(async function test_deprecated_LWT_properties_ignored() {
   );
 
   Assert.equal(
-    style.backgroundColor,
+    navigatorStyle.backgroundColor,
     DEFAULT_THEME_BG_COLOR,
     "Expected default theme background color"
   );
   Assert.equal(
-    style.color,
+    docStyle.color,
     DEFAULT_THEME_TEXT_COLOR,
     "Expected default theme text color"
   );

toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js

@@ -40,28 +40,27 @@ add_task(async function test_support_backgrounds_position() {
   );
 
   let toolboxCS = window.getComputedStyle(toolbox);
-  let rootCS = window.getComputedStyle(docEl);
-  let rootBgImage = rootCS.backgroundImage.split(",")[0].trim();
   let bgImage = toolboxCS.backgroundImage.split(",")[0].trim();
-  Assert.ok(
-    rootBgImage.includes("face1.png"),
-    `The backgroundImage should use face1.png. Actual value is: ${rootBgImage}`
-  );
   Assert.equal(
     toolboxCS.backgroundImage,
-    Array(3)
-      .fill(bgImage)
+    [1, 2, 2, 2]
+      .map(num => bgImage.replace(/face[\d]*/, `face${num}`))
       .join(", "),
-    "The backgroundImage should use face2.png three times."
+    "The backgroundImage should use face1.png once and face2.png three times."
   );
   Assert.equal(
     toolboxCS.backgroundPosition,
-    "0% 0%, 50% 0%, 100% 100%",
-    "The backgroundPosition should use the three values provided."
+    "100% 0%, 0% 0%, 50% 0%, 100% 100%",
+    "The backgroundPosition should use the three values provided, preceded by the default for theme_frame."
   );
+  /**
+   * We expect duplicate background-repeat values because we apply `no-repeat`
+   * once for theme_frame, and again as the default value of
+   * --lwt-background-tiling.
+   */
   Assert.equal(
     toolboxCS.backgroundRepeat,
-    "no-repeat",
+    "no-repeat, no-repeat",
     "The backgroundPosition should use the default value."
   );
 
@@ -71,9 +70,6 @@ add_task(async function test_support_backgrounds_position() {
   toolboxCS = window.getComputedStyle(toolbox);
 
   // Styles should've reverted to their initial values.
-  Assert.equal(rootCS.backgroundImage, "none");
-  Assert.equal(rootCS.backgroundPosition, "0% 0%");
-  Assert.equal(rootCS.backgroundRepeat, "repeat");
   Assert.equal(toolboxCS.backgroundImage, "none");
   Assert.equal(toolboxCS.backgroundPosition, "0% 0%");
   Assert.equal(toolboxCS.backgroundRepeat, "repeat");
@@ -116,37 +112,29 @@ add_task(async function test_support_backgrounds_repeat() {
     "LWT text color attribute should be set"
   );
 
-  let rootCS = window.getComputedStyle(docEl);
   let toolboxCS = window.getComputedStyle(toolbox);
-  let bgImage = rootCS.backgroundImage.split(",")[0].trim();
-  Assert.ok(
-    bgImage.includes("face0.png"),
-    `The backgroundImage should use face.png. Actual value is: ${bgImage}`
-  );
+  let bgImage = toolboxCS.backgroundImage.split(",")[0].trim();
   Assert.equal(
-    [1, 2, 3].map(num => bgImage.replace(/face[\d]*/, `face${num}`)).join(", "),
+    [0, 1, 2, 3]
+      .map(num => bgImage.replace(/face[\d]*/, `face${num}`))
+      .join(", "),
     toolboxCS.backgroundImage,
-    "The backgroundImage should use face.png three times."
-  );
-  Assert.equal(
-    rootCS.backgroundPosition,
-    "100% 0%",
-    "The backgroundPosition should use the default value for root."
+    "The backgroundImage should use face.png four times."
   );
+  /**
+   * We expect duplicate background-position values because we apply `right top`
+   * once for theme_frame, and again as the default value of
+   * --lwt-background-alignment.
+   */
   Assert.equal(
     toolboxCS.backgroundPosition,
-    "100% 0%",
+    "100% 0%, 100% 0%",
     "The backgroundPosition should use the default value for navigator-toolbox."
   );
-  Assert.equal(
-    rootCS.backgroundRepeat,
-    "no-repeat",
-    "The backgroundRepeat should use the default values for root."
-  );
   Assert.equal(
     toolboxCS.backgroundRepeat,
-    "repeat-x, repeat-y, repeat",
-    "The backgroundRepeat should use the three values provided for navigator-toolbox."
+    "no-repeat, repeat-x, repeat-y, repeat",
+    "The backgroundRepeat should use the three values provided for --lwt-background-tiling, preceeded by the default for theme_frame."
   );
 
   await extension.unload();
@@ -187,27 +175,21 @@ add_task(async function test_additional_images_check() {
     "LWT text color attribute should be set"
   );
 
-  let rootCS = window.getComputedStyle(docEl);
   let toolboxCS = window.getComputedStyle(toolbox);
-  let bgImage = rootCS.backgroundImage.split(",")[0];
+  let bgImage = toolboxCS.backgroundImage.split(",")[0];
   Assert.ok(
     bgImage.includes("face.png"),
     `The backgroundImage should use face.png. Actual value is: ${bgImage}`
   );
   Assert.equal(
-    "none",
-    toolboxCS.backgroundImage,
-    "The backgroundImage should not use face.png."
-  );
-  Assert.equal(
-    rootCS.backgroundPosition,
-    "100% 0%",
+    toolboxCS.backgroundPosition,
+    "100% 0%, 100% 0%",
     "The backgroundPosition should use the default value."
   );
   Assert.equal(
-    rootCS.backgroundRepeat,
-    "no-repeat",
-    "The backgroundPosition should use only one (default) value."
+    toolboxCS.backgroundRepeat,
+    "no-repeat, no-repeat",
+    "The backgroundRepeat should use the default value."
   );
 
   await extension.unload();

toolkit/components/extensions/test/browser/browser_ext_themes_persistence.js

@@ -24,7 +24,8 @@ add_task(async function test_multiple_windows() {
   await extension.startup();
 
   let docEl = window.document.documentElement;
-  let style = window.getComputedStyle(docEl);
+  let toolbox = document.querySelector("#navigator-toolbox");
+  let toolboxCS = window.getComputedStyle(toolbox);
 
   Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
   Assert.equal(
@@ -33,14 +34,15 @@ add_task(async function test_multiple_windows() {
     "LWT text color attribute should be set"
   );
   Assert.ok(
-    style.backgroundImage.includes("image1.png"),
+    toolboxCS.backgroundImage.includes("image1.png"),
     "Expected background image"
   );
 
   // Now we'll open a new window to see if the theme is also applied there.
   let window2 = await BrowserTestUtils.openNewBrowserWindow();
   docEl = window2.document.documentElement;
-  style = window2.getComputedStyle(docEl);
+  toolbox = window2.document.querySelector("#navigator-toolbox");
+  toolboxCS = window2.getComputedStyle(toolbox);
 
   Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
   Assert.equal(
@@ -49,7 +51,7 @@ add_task(async function test_multiple_windows() {
     "LWT text color attribute should be set"
   );
   Assert.ok(
-    style.backgroundImage.includes("image1.png"),
+    toolboxCS.backgroundImage.includes("image1.png"),
     "Expected background image"
   );
 

toolkit/components/extensions/test/browser/browser_ext_themes_sanitization.js

@@ -137,9 +137,11 @@ add_task(async function test_sanitization_transparent_frame_color() {
 
   await extension.startup();
 
-  let docEl = document.documentElement;
+  let toolbox = document.querySelector("#navigator-toolbox");
+  let toolboxCS = window.getComputedStyle(toolbox);
+
   Assert.equal(
-    window.getComputedStyle(docEl).backgroundColor,
+    toolboxCS.backgroundColor,
     "rgb(255, 255, 255)",
     "Accent color should be white"
   );

toolkit/themes/osx/global/global.css

@@ -75,10 +75,6 @@ xul|iframe {
 
 /* ::::: Miscellaneous formatting ::::: */
 
-:root:-moz-lwtheme {
-  appearance: none;
-}
-
 :root[lwtheme-image]:-moz-lwtheme-darktext {
   text-shadow: 0 -0.5px 1.5px white;
 }