Bug 1472740 - Remove support for deprecated lwt aliases from WebExtensions theme API. r=ntim,robwu

Removing the lwt aliases from the theme API schema is not going to be enough, because the images and colors properties are very permissive on the unknown properties (likely to prevent a property supported on chrome but not on firefox to prevent the theme from being installed), and so removing the lwt aliases from the schema would not raise any error (the theme API implementation would just be silently ignoring the deprecated lwt aliases).

For the above reason the following patch use the following approach:

- kept the deprecated lwt aliases in the schema, but changes the deprecation warning message to mention that the property is now completely ignored by Firefox, and which property should be used instead

- removed the deprecation warning from the toolbar_text theme colors property, as we decided that we are not going to deprecate it anymore

- changed the theme API implementation to ignore the deprecated lwt alias property

- repurposed browser_ext_themes_lwtsupport.js test file to verify that the lwt aliases are ignored as expected

A separate addons-linter pull request is going to be created, to ensure that the addons-linter will raise linting errors (instead of linting warning) when these deprecated lwt aliases are being used in a theme (to prevent that newly submitted theme versions including the aliases will go unnoticed).

Depends on D37890

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

--HG--
extra : moz-landing-system : lando

toolkit/components/extensions/parent/ext-theme.js

@@ -176,14 +176,12 @@ class Theme {
       }
 
       switch (color) {
-        case "accentcolor":
         case "frame":
           styles.accentcolor = cssColor;
           break;
         case "frame_inactive":
           styles.accentcolorInactive = cssColor;
           break;
-        case "textcolor":
         case "tab_background_text":
           styles.textcolor = cssColor;
           break;
@@ -271,7 +269,6 @@ class Theme {
           styles.additionalBackgrounds = backgroundImages;
           break;
         }
-        case "headerURL":
         case "theme_frame": {
           let resolvedURL = baseURI.resolve(val);
           styles.headerURL = resolvedURL;

toolkit/components/extensions/schemas/theme.json

@@ -89,7 +89,7 @@
               "headerURL": {
                 "$ref": "ImageDataOrExtensionURL",
                 "optional": true,
-                "deprecated": "Please use <em>theme.images.theme_frame</em>, this alias will be removed in Firefox 69."
+                "deprecated": "Unsupported images property, use 'theme.images.theme_frame', this alias is ignored in Firefox >= 70."
               },
               "theme_frame": {
                 "$ref": "ImageDataOrExtensionURL",
@@ -109,7 +109,7 @@
               "accentcolor": {
                 "$ref": "ThemeColor",
                 "optional": true,
-                "deprecated": "Please use <em>theme.colors.frame</em>, this alias will be removed in Firefox 69."
+                "deprecated": "Unsupported colors property, use 'theme.colors.frame', this alias is ignored in Firefox >= 70."
               },
               "frame": {
                 "$ref": "ThemeColor",
@@ -122,7 +122,7 @@
               "textcolor": {
                 "$ref": "ThemeColor",
                 "optional": true,
-                "deprecated": "Please use <em>theme.colors.tab_background_text</em>, this alias will be removed in Firefox 69."
+                "deprecated": "Unsupported color property, use 'theme.colors.tab_background_text', this alias is ignored in Firefox >= 70."
               },
               "tab_background_text": {
                 "$ref": "ThemeColor",
@@ -151,7 +151,7 @@
               "toolbar_text": {
                 "$ref": "ThemeColor",
                 "optional": true,
-                "deprecated": "Please use <em>theme.colors.bookmark_text</em>, this alias will be removed in Firefox 69."
+                 "description": "This color property is an alias of 'bookmark_text'."
               },
               "bookmark_text": {
                 "$ref": "ThemeColor",

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

@@ -2,7 +2,7 @@
 
 add_task(async function test_support_theme_frame() {
   const FRAME_COLOR = [71, 105, 91];
-  const TAB_TEXT_COLOR = [207, 221, 192];
+  const TAB_TEXT_COLOR = [0, 0, 0];
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       theme: {
@@ -24,9 +24,15 @@ add_task(async function test_support_theme_frame() {
 
   let docEl = window.document.documentElement;
   Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
+
+  Assert.ok(
+    docEl.hasAttribute("lwtheme-image"),
+    "LWT image attribute should be set"
+  );
+
   Assert.equal(
     docEl.getAttribute("lwthemetextcolor"),
-    "bright",
+    "dark",
     "LWT text color attribute should be set"
   );
 
@@ -51,6 +57,16 @@ add_task(async function test_support_theme_frame() {
   await extension.unload();
 
   Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
+
+  Assert.ok(
+    !docEl.hasAttribute("lwtheme-image"),
+    "LWT image attribute should not be set"
+  );
+
+  Assert.ok(
+    !docEl.hasAttribute("lwthemetextcolor"),
+    "LWT text color attribute should not be set"
+  );
 });
 
 add_task(async function test_support_theme_frame_inactive() {

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

@@ -1,6 +1,9 @@
 "use strict";
 
-add_task(async function test_support_LWT_properties() {
+const DEFAULT_THEME_BG_COLOR = "rgb(255, 255, 255)";
+const DEFAULT_THEME_TEXT_COLOR = "rgb(0, 0, 0)";
+
+add_task(async function test_deprecated_LWT_properties_ignored() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       theme: {
@@ -25,120 +28,27 @@ add_task(async function test_support_LWT_properties() {
 
   Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
   Assert.ok(
-    docEl.hasAttribute("lwtheme-image"),
-    "LWT image attribute should be set"
+    !docEl.hasAttribute("lwtheme-image"),
+    "LWT image attribute should not be set on deprecated headerURL alias"
   );
   Assert.equal(
     docEl.getAttribute("lwthemetextcolor"),
-    "bright",
-    "LWT text color attribute should be set"
+    "dark",
+    "LWT text color attribute should not be set on deprecated textcolor alias"
   );
 
-  Assert.ok(
-    style.backgroundImage.includes("image1.png"),
-    "Expected background image"
-  );
   Assert.equal(
     style.backgroundColor,
-    "rgb(" + hexToRGB(ACCENT_COLOR).join(", ") + ")",
-    "Expected correct background color"
+    DEFAULT_THEME_BG_COLOR,
+    "Expected default theme background color"
   );
   Assert.equal(
     style.color,
-    "rgb(" + hexToRGB(TEXT_COLOR).join(", ") + ")",
-    "Expected correct text color"
+    DEFAULT_THEME_TEXT_COLOR,
+    "Expected default theme text color"
   );
 
   await extension.unload();
 
   Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
-  Assert.ok(
-    !docEl.hasAttribute("lwtheme-image"),
-    "LWT image attribute should not be set"
-  );
-});
-
-add_task(async function test_LWT_image_attribute() {
-  let extension = ExtensionTestUtils.loadExtension({
-    manifest: {
-      theme: {
-        colors: {
-          accentcolor: ACCENT_COLOR,
-          textcolor: TEXT_COLOR,
-        },
-      },
-    },
-  });
-
-  await extension.startup();
-
-  let docEl = window.document.documentElement;
-  Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
-  Assert.ok(
-    !docEl.hasAttribute("lwtheme-image"),
-    "LWT image attribute should not be set"
-  );
-  await extension.unload();
-  Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
-  Assert.ok(
-    !docEl.hasAttribute("lwtheme-image"),
-    "LWT image attribute should not be set"
-  );
-});
-
-add_task(async function test_LWT_does_not_require_accentcolor_textcolor_only() {
-  let extension = ExtensionTestUtils.loadExtension({
-    manifest: {
-      theme: {
-        colors: {
-          textcolor: TEXT_COLOR,
-        },
-      },
-    },
-  });
-
-  await extension.startup();
-
-  let docEl = window.document.documentElement;
-  Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
-  Assert.ok(
-    !docEl.hasAttribute("lwtheme-image"),
-    "LWT image attribute should not be set"
-  );
-  await extension.unload();
-  Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
-  Assert.ok(
-    !docEl.hasAttribute("lwtheme-image"),
-    "LWT image attribute should not be set"
-  );
-});
-
-add_task(async function test_LWT_does_not_require_accentcolor_image_only() {
-  let extension = ExtensionTestUtils.loadExtension({
-    manifest: {
-      theme: {
-        images: {
-          headerURL: "image1.png",
-        },
-      },
-    },
-    files: {
-      "image1.png": BACKGROUND,
-    },
-  });
-
-  await extension.startup();
-
-  let docEl = window.document.documentElement;
-  Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set");
-  Assert.ok(
-    docEl.hasAttribute("lwtheme-image"),
-    "LWT image attribute should be set"
-  );
-  await extension.unload();
-  Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set");
-  Assert.ok(
-    !docEl.hasAttribute("lwtheme-image"),
-    "LWT image attribute should not be set"
-  );
 });

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

@@ -13,8 +13,6 @@ add_task(async function test_support_toolbar_property() {
           frame: ACCENT_COLOR,
           tab_background_text: TEXT_COLOR,
           toolbar: TOOLBAR_COLOR,
-          // NOTE: this property is going to be removed on Firefox 69
-          // (and bookmark_text is going to replace it).
           toolbar_text: TOOLBAR_TEXT_COLOR,
         },
       },