Bug 1404568 - Improve webext browser_action icon fallbacks. r=mixedpuppy, a=ritu
authorIan Moody <moz-ian@perix.co.uk>
Sun, 08 Oct 2017 13:23:33 +0100
changeset 432379 99d96c2d86364a69b9e70f510563580a1b1da2a4
parent 432378 63c9ad5b7e93b1dbb069cfc0f18587ece24bf474
child 432380 bbda3799f0c8bd4b0665c10b5765dd7c457a5ada
push id7947
push userryanvm@gmail.com
push dateWed, 11 Oct 2017 18:32:48 +0000
treeherdermozilla-beta@2220c5f79afb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy, ritu
bugs1404568
milestone57.0
Bug 1404568 - Improve webext browser_action icon fallbacks. r=mixedpuppy, a=ritu Currently if there is no default icon at the specified size, the default icon falls back to the light text icon at that size. This is wrong in two ways: First, the default theme uses dark text, so it should fallback to the dark icon Secondly, authors expect the unsized default_icon to be used if specified This patch fixes both of these issues, so that the default icon first falls back to the unsized default_icon, and then only if that is not specified falls back to the dark icon MozReview-Commit-ID: C3RRTKhYq6r
browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js
toolkit/components/extensions/ExtensionParent.jsm
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js
@@ -41,17 +41,17 @@ async function testStaticTheme(options) 
     manifest.browser_action.default_icon = "default.png";
   }
 
   let extension = ExtensionTestUtils.loadExtension({manifest});
 
   await extension.startup();
 
   // Confirm that the browser action has the correct default icon before a theme is loaded.
-  let expectedDefaultIcon = withDefaultIcon ? "default.png" : "light.png";
+  let expectedDefaultIcon = withDefaultIcon ? "default.png" : "dark.png";
   await testBrowserAction(extension, expectedDefaultIcon);
 
   let theme = ExtensionTestUtils.loadExtension({
     manifest: {
       "theme": {
         "images": {
           "headerURL": "background.png",
         },
@@ -159,17 +159,17 @@ add_task(async function browseraction_th
 
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "browser_action": {
         "default_icon": "default.png",
         "theme_icons": [{
           "light": "light.png",
           "dark": "dark.png",
-          "size": 19,
+          "size": 16,
         }],
       },
     },
   });
 
   await extension.startup();
 
   // Confirm that the browser action has the default icon before a theme is set.
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -1301,19 +1301,19 @@ let IconDetails = {
       if (themeIcons) {
         themeIcons.forEach(({size, light, dark}) => {
           let lightURL = baseURI.resolve(light);
           let darkURL = baseURI.resolve(dark);
 
           this._checkURL(lightURL, extension);
           this._checkURL(darkURL, extension);
 
-          let defaultURL = result[size];
+          let defaultURL = result[size] || result[19]; // always fallback to default first
           result[size] = {
-            "default": defaultURL || lightURL, // Fallback to the light url if no default is specified.
+            "default": defaultURL || darkURL, // Fallback to the dark url if no default is specified.
             "light": lightURL,
             "dark": darkURL,
           };
         });
       }
     } catch (e) {
       // Function is called from extension code, delegate error.
       if (context) {