Bug 1352120 - fix theming for the star icon, fix theming dealing with empty string icon urls, r=jaws

When debugging the test failures in this test, I noticed that the info() messages indicated we *were* using
moz-extension icon references even when we shouldn't be - they just didn't include the 'fox.svg' bit.
When pausing in the debugger, you can see that all the buttons are blank - we don't load any icon in this case.

This seemed bad, so I updated the test to actually check if we're using a moz-extension URI at all,
and then updated the implementation to actually make it work.

MozReview-Commit-ID: GGXaivJrzxj

--HG--
extra : rebase_source : a64bed37e1fb27c164a0543a0465038d251c709d

browser/base/content/theme-vars.inc.css

@@ -20,10 +20,16 @@
   list-style-image: var(--stop-icon) !important;
 }
 
+%ifdef MOZ_PHOTON_THEME
+:root[lwthemeicons~="--bookmark_star-icon"] #star-button:-moz-lwtheme,
+%endif
 :root[lwthemeicons~="--bookmark_star-icon"] #bookmarks-menu-button:-moz-lwtheme {
   list-style-image: var(--bookmark_star-icon) !important;
 }
 
+%ifdef MOZ_PHOTON_THEME
+:root[lwthemeicons~="--bookmark_menu-icon"] #bookmarks-menu-button:-moz-lwtheme,
+%endif
 :root[lwthemeicons~="--bookmark_menu-icon"] #bookmarks-menu-button[cui-areatype='toolbar'] > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon:-moz-lwtheme {
   list-style-image: var(--bookmark_menu-icon) !important;
 }
@@ -128,6 +134,10 @@
   list-style-image: var(--pocket-icon) !important;
 }
 
+%ifdef MOZ_PHOTON_THEME
+:root[lwthemeicons~="--bookmark_star-icon"] #star-button:-moz-lwtheme,
+:root[lwthemeicons~="--bookmark_menu-icon"] #bookmarks-menu-button:-moz-lwtheme,
+%endif
 :root[lwthemeicons~="--back-icon"] #back-button:-moz-lwtheme,
 :root[lwthemeicons~="--forward-icon"] #forward-button:-moz-lwtheme,
 :root[lwthemeicons~="--reload-icon"] #reload-button:-moz-lwtheme,

browser/components/extensions/test/browser/browser_ext_themes_icons.js

@@ -32,7 +32,7 @@ function verifyButtonProperties(selector, shouldHaveCustomStyling, message) {
 
     let listStyleImage = getComputedStyle(element).listStyleImage;
     info(`listStyleImage for fox.svg is ${listStyleImage}`);
-    is(listStyleImage.includes("fox.svg"), shouldHaveCustomStyling, message);
+    is(listStyleImage.includes("moz-extension:"), shouldHaveCustomStyling, message);
   } catch (ex) {
     ok(false, `Unable to verify ${selector}: ${ex}`);
   }
@@ -77,10 +77,10 @@ function checkButtons(icons, iconInfo, area) {
     let iconInfo = icons.find(arr => arr[0] == button[0]);
     if (iconInfo[1]) {
       verifyButtonWithCustomStyling(button[1],
-        `The ${button[1]} should have it's icon customized in the ${area}`);
+        `The ${button[1]} should have its icon customized in the ${area}`);
     } else {
       verifyButtonWithoutCustomStyling(button[1],
-        `The ${button[1]} should not have it's icon customized in the ${area}`);
+        `The ${button[1]} should not have its icon customized in the ${area}`);
     }
   }
 }
@@ -114,8 +114,6 @@ async function runTestWithIcons(icons) {
     ["forward", "#forward-button"],
     ["reload", "#reload-button"],
     ["stop", "#stop-button"],
-    ["bookmark_star", "#bookmarks-menu-button", "bookmarks-menu-button"],
-    ["bookmark_menu", "#bookmarks-menu-button > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon"],
     ["downloads", "#downloads-button", "downloads-button"],
     ["home", "#home-button", "home-button"],
     ["app_menu", "#PanelUI-menu-button"],
@@ -142,6 +140,13 @@ async function runTestWithIcons(icons) {
     ["forget", "#panic-button", "panic-button"],
     ["pocket", "#pocket-button", "pocket-button"],
   ];
+  if (AppConstants.MOZ_PHOTON_THEME) {
+    ICON_INFO.push(["bookmark_star", "#star-button"]);
+    ICON_INFO.push(["bookmark_menu", "#bookmarks-menu-button", "bookmarks-menu-button"]);
+  } else {
+    ICON_INFO.push(["bookmark_star", "#bookmarks-menu-button", "bookmarks-menu-button"]);
+    ICON_INFO.push(["bookmark_menu", "#bookmarks-menu-button > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon"]);
+  }
 
   window.maximize();
 
@@ -151,7 +156,7 @@ async function runTestWithIcons(icons) {
     }
 
     verifyButtonWithoutCustomStyling(button[1],
-      `The ${button[1]} should not have it's icon customized when the test starts`);
+      `The ${button[1]} should not have its icon customized when the test starts`);
 
     let iconInfo = icons.find(arr => arr[0] == button[0]);
     manifest.theme.icons[button[0]] = iconInfo[1];
@@ -181,7 +186,7 @@ async function runTestWithIcons(icons) {
 
   for (let button of ICON_INFO) {
     verifyButtonWithoutCustomStyling(button[1],
-      `The ${button[1]} should not have it's icon customized when the theme is unloaded`);
+      `The ${button[1]} should not have its icon customized when the theme is unloaded`);
   }
 }
 

toolkit/components/extensions/ext-theme.js

@@ -140,7 +140,10 @@ class Theme {
 
     for (let icon of Object.getOwnPropertyNames(icons)) {
       let val = icons[icon];
-      if (!val || !ICONS.includes(icon)) {
+      // We also have to compare against the baseURI spec because
+      // `val` might have been resolved already. Resolving "" against
+      // the baseURI just produces that URI, so check for equality.
+      if (!val || val == this.baseURI.spec || !ICONS.includes(icon)) {
         continue;
       }
       let variableName = `--${icon}-icon`;