Bug 1272222: Use larger icons for browser actions in the menu panel. r=Gijs

MozReview-Commit-ID: 26lmlcrngPk

--HG--
extra : rebase_source : b9d6bfb7669d3cb826ccaa6728e153ad3e3b2b8b

browser/base/content/browser.css

@@ -327,6 +327,36 @@ toolbarpaletteitem > toolbaritem[sdkstylewidget="true"][cui-areatype="toolbar"]
   display: -moz-box;
 }
 
+@media not all and (min-resolution: 1.1dppx) {
+  .webextension-browser-action {
+    list-style-image: var(--webextension-toolbar-image);
+  }
+
+  .webextension-browser-action[cui-areatype="menu-panel"],
+  toolbarpaletteitem[place="palette"] > .webextension-browser-action {
+    list-style-image: var(--webextension-menupanel-image);
+  }
+
+  .webextension-page-action {
+    list-style-image: var(--webextension-urlbar-image);
+  }
+}
+
+@media (min-resolution: 1.1dppx) {
+  .webextension-browser-action {
+    list-style-image: var(--webextension-toolbar-image-2x);
+  }
+
+  .webextension-browser-action[cui-areatype="menu-panel"],
+  toolbarpaletteitem[place="palette"] > .webextension-browser-action {
+    list-style-image: var(--webextension-menupanel-image-2x);
+  }
+
+  .webextension-page-action {
+    list-style-image: var(--webextension-urlbar-image-2x);
+  }
+}
+
 toolbarpaletteitem[removable="false"] {
   opacity: 0.5;
   cursor: default;

browser/components/extensions/ext-browserAction.js

@@ -79,6 +79,7 @@ BrowserAction.prototype = {
 
       onCreated: node => {
         node.classList.add("badged-button");
+        node.classList.add("webextension-browser-action");
         node.setAttribute("constrain-size", "true");
 
         this.updateButton(node, this.defaults);
@@ -150,22 +151,34 @@ BrowserAction.prototype = {
     const LEGACY_CLASS = "toolbarbutton-legacy-addon";
     node.classList.remove(LEGACY_CLASS);
 
-
-    let win = node.ownerDocument.defaultView;
-    let {icon, size} = IconDetails.getURL(tabData.icon, win, this.extension);
+    let baseSize = 16;
+    let {icon, size} = IconDetails.getPreferredIcon(tabData.icon, this.extension, baseSize);
 
     // If the best available icon size is not divisible by 16, check if we have
     // an 18px icon to fall back to, and trim off the padding instead.
     if (size % 16 && !icon.endsWith(".svg")) {
-      let result = IconDetails.getURL(tabData.icon, win, this.extension, 18);
+      let result = IconDetails.getPreferredIcon(tabData.icon, this.extension, 18);
 
       if (result.size % 18 == 0) {
+        baseSize = 18;
         icon = result.icon;
         node.classList.add(LEGACY_CLASS);
       }
     }
 
-    node.setAttribute("image", icon);
+    // These URLs should already be properly escaped, but make doubly sure CSS
+    // string escape characters are escaped here, since they could lead to a
+    // sandbox break.
+    let escape = str => str.replace(/[\\\s"]/g, encodeURIComponent);
+
+    let getIcon = size => escape(IconDetails.getPreferredIcon(tabData.icon, this.extension, size).icon);
+
+    node.setAttribute("style", `
+      --webextension-menupanel-image: url("${getIcon(32)}");
+      --webextension-menupanel-image-2x: url("${getIcon(64)}");
+      --webextension-toolbar-image: url("${escape(icon)}");
+      --webextension-toolbar-image-2x: url("${getIcon(baseSize * 2)}");
+    `);
   },
 
   // Update the toolbar button for a given window.

browser/components/extensions/ext-contextMenus.js

@@ -54,7 +54,12 @@ var gMenuBuilder = {
         let parentWindow = contextData.menu.ownerDocument.defaultView;
         let extension = root.extension;
 
-        let {icon} = IconDetails.getURL(extension.manifest.icons, parentWindow, extension, 16 /* size */);
+        let {icon} = IconDetails.getPreferredIcon(extension.manifest.icons, extension,
+                                                  16 * parentWindow.devicePixelRatio);
+
+        // The extension icons in the manifest are not pre-resolved, since
+        // they're sometimes used by the add-on manager when the extension is
+        // not enabled, and its URLs are not resolvable.
         let resolvedURL = root.extension.baseURI.resolve(icon);
 
         if (rootElement.localName == "menu") {

browser/components/extensions/ext-pageAction.js

@@ -92,8 +92,19 @@ PageAction.prototype = {
       button.setAttribute("tooltiptext", title);
       button.setAttribute("aria-label", title);
 
-      let {icon} = IconDetails.getURL(tabData.icon, window, this.extension);
-      button.setAttribute("src", icon);
+      // These URLs should already be properly escaped, but make doubly sure CSS
+      // string escape characters are escaped here, since they could lead to a
+      // sandbox break.
+      let escape = str => str.replace(/[\\\s"]/g, encodeURIComponent);
+
+      let getIcon = size => escape(IconDetails.getPreferredIcon(tabData.icon, this.extension, size).icon);
+
+      button.setAttribute("style", `
+        --webextension-urlbar-image: url("${getIcon(16)}");
+        --webextension-urlbar-image-2x: url("${getIcon(32)}");
+      `);
+
+      button.classList.add("webextension-page-action");
     }
 
     button.hidden = !tabData.show;

browser/components/extensions/test/browser/browser_ext_browserAction_context.js

@@ -97,7 +97,7 @@ function* runTests(options) {
 
     let title = details.title || options.manifest.name;
 
-    is(button.getAttribute("image"), details.icon, "icon URL is correct");
+    is(getListStyleImage(button), details.icon, "icon URL is correct");
     is(button.getAttribute("tooltiptext"), title, "image title is correct");
     is(button.getAttribute("label"), title, "image label is correct");
     is(button.getAttribute("badge"), details.badge, "badge text is correct");
@@ -163,6 +163,11 @@ add_task(function* testTabSwitchContext() {
           "description": "Title",
         },
       },
+
+      "default.png": imageBuffer,
+      "default-2.png": imageBuffer,
+      "1.png": imageBuffer,
+      "2.png": imageBuffer,
     },
 
     getTests(tabs, expectDefaults) {
@@ -322,6 +327,10 @@ add_task(function* testDefaultTitle() {
       "permissions": ["tabs"],
     },
 
+    files: {
+      "icon.png": imageBuffer,
+    },
+
     getTests(tabs, expectDefaults) {
       let details = [
         {"title": "Foo Extension",

browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js

@@ -52,6 +52,12 @@ add_task(function* testDetailsObjects() {
           "1": browser.runtime.getURL("data/a.png"),
           "2": browser.runtime.getURL("data/a-x2.png")}},
 
+      // Test that CSS strings are escaped properly.
+      {details: {"path": 'a.png#" \\'},
+        resolutions: {
+          "1": browser.runtime.getURL("data/a.png#%22%20%5C"),
+          "2": browser.runtime.getURL("data/a.png#%22%20%5C")}},
+
       // Only ImageData objects.
       {details: {"imageData": imageData.red.imageData},
         resolutions: {
@@ -136,16 +142,23 @@ add_task(function* testDetailsObjects() {
         legacy: true,
         resolutions: {
           "1": browser.runtime.getURL("data/18.png"),
-          "2": browser.runtime.getURL("data/36.png")}},
+          "2": browser.runtime.getURL("data/36.png")},
+        menuResolutions: {
+          "1": browser.runtime.getURL("data/36.png"),
+          "2": browser.runtime.getURL("data/128.png")}},
       {details: {"path": {
         "16": "16.png",
         "18": "18.png",
         "32": "32.png",
         "48": "48.png",
+        "64": "64.png",
         "128": "128.png"}},
         resolutions: {
           "1": browser.runtime.getURL("data/16.png"),
-          "2": browser.runtime.getURL("data/32.png")}},
+          "2": browser.runtime.getURL("data/32.png")},
+        menuResolutions: {
+          "1": browser.runtime.getURL("data/32.png"),
+          "2": browser.runtime.getURL("data/64.png")}},
       {details: {"path": {
         "18": "18.png",
         "32": "32.png",
@@ -167,15 +180,14 @@ add_task(function* testDetailsObjects() {
       }
 
       let details = iconDetails[test.index];
-      let expectedURL = details.resolutions[test.resolution];
 
       let detailString = JSON.stringify(details);
-      browser.test.log(`Setting browerAction/pageAction to ${detailString} expecting URL ${expectedURL}`);
+      browser.test.log(`Setting browerAction/pageAction to ${detailString} expecting URLs ${JSON.stringify(details.resolutions)}`);
 
       browser.browserAction.setIcon(Object.assign({tabId}, details.details));
       browser.pageAction.setIcon(Object.assign({tabId}, details.details));
 
-      browser.test.sendMessage("imageURL", [expectedURL, !!details.legacy]);
+      browser.test.sendMessage("iconSet");
     });
 
     // Generate a list of tests and resolutions to send back to the test
@@ -190,9 +202,12 @@ add_task(function* testDetailsObjects() {
     // correctly.
     let tests = [];
     for (let [idx, icon] of iconDetails.entries()) {
-      for (let res of Object.keys(icon.resolutions)) {
-        tests.push({index: idx, resolution: Number(res)});
-      }
+      tests.push({
+        index: idx,
+        legacy: !!icon.legacy,
+        menuResolutions: icon.menuResolutions,
+        resolutions: icon.resolutions,
+      });
     }
 
     // Sort by resolution, so we don't needlessly switch back and forth
@@ -219,35 +234,86 @@ add_task(function* testDetailsObjects() {
     files: {
       "data/background.html": `<script src="background.js"></script>`,
       "data/background.js": background,
+
+      "data/16.svg": imageBuffer,
+      "data/18.svg": imageBuffer,
+
+      "data/16.png": imageBuffer,
+      "data/18.png": imageBuffer,
+      "data/32.png": imageBuffer,
+      "data/36.png": imageBuffer,
+      "data/48.png": imageBuffer,
+      "data/64.png": imageBuffer,
+      "data/128.png": imageBuffer,
+
+      "a.png": imageBuffer,
+      "data/2.png": imageBuffer,
+      "data/100.png": imageBuffer,
+      "data/a.png": imageBuffer,
+      "data/a-x2.png": imageBuffer,
     },
   });
 
   const RESOLUTION_PREF = "layout.css.devPixelsPerPx";
-  registerCleanupFunction(() => {
-    SpecialPowers.clearUserPref(RESOLUTION_PREF);
-  });
 
-  let browserActionId = makeWidgetId(extension.id) + "-browser-action";
   let pageActionId = makeWidgetId(extension.id) + "-page-action";
+  let browserActionWidget = getBrowserActionWidget(extension);
 
-  let [, tests] = yield Promise.all([extension.startup(), extension.awaitMessage("ready")]);
+
+  yield extension.startup();
+  let tests = yield extension.awaitMessage("ready");
 
   for (let test of tests) {
-    SpecialPowers.setCharPref(RESOLUTION_PREF, String(test.resolution));
-    is(window.devicePixelRatio, test.resolution, "window has the required resolution");
-
     extension.sendMessage("setIcon", test);
+    yield extension.awaitMessage("iconSet");
 
-    let [imageURL, legacy] = yield extension.awaitMessage("imageURL");
-
-    let browserActionButton = document.getElementById(browserActionId);
-    is(browserActionButton.getAttribute("image"), imageURL, "browser action has the correct image");
-
-    let isLegacy = browserActionButton.classList.contains("toolbarbutton-legacy-addon");
-    is(isLegacy, legacy, "Legacy class should be present?");
-
+    let browserActionButton = browserActionWidget.forWindow(window).node;
     let pageActionImage = document.getElementById(pageActionId);
-    is(pageActionImage.src, imageURL, "page action has the correct image");
+
+
+    // Test icon sizes in the toolbar/urlbar.
+    for (let resolution of Object.keys(test.resolutions)) {
+      yield SpecialPowers.pushPrefEnv({set: [[RESOLUTION_PREF, resolution]]});
+
+      is(window.devicePixelRatio, +resolution, "window has the required resolution");
+
+      let imageURL = test.resolutions[resolution];
+      is(getListStyleImage(browserActionButton), imageURL, `browser action has the correct image at ${resolution}x resolution`);
+      is(getListStyleImage(pageActionImage), imageURL, `page action has the correct image at ${resolution}x resolution`);
+
+      let isLegacy = browserActionButton.classList.contains("toolbarbutton-legacy-addon");
+      is(isLegacy, test.legacy, "Legacy class should be present?");
+
+      yield SpecialPowers.popPrefEnv();
+    }
+
+    if (!test.menuResolutions) {
+      continue;
+    }
+
+
+    // Test icon sizes in the menu panel.
+    CustomizableUI.addWidgetToArea(browserActionWidget.id,
+                                   CustomizableUI.AREA_PANEL);
+
+    yield showBrowserAction(extension);
+    browserActionButton = browserActionWidget.forWindow(window).node;
+
+    for (let resolution of Object.keys(test.menuResolutions)) {
+      yield SpecialPowers.pushPrefEnv({set: [[RESOLUTION_PREF, resolution]]});
+
+      is(window.devicePixelRatio, +resolution, "window has the required resolution");
+
+      let imageURL = test.menuResolutions[resolution];
+      is(getListStyleImage(browserActionButton), imageURL, `browser action has the correct menu image at ${resolution}x resolution`);
+
+      yield SpecialPowers.popPrefEnv();
+    }
+
+    yield closeBrowserAction(extension);
+
+    CustomizableUI.addWidgetToArea(browserActionWidget.id,
+                                   CustomizableUI.AREA_NAVBAR);
   }
 
   yield extension.unload();
@@ -341,7 +407,12 @@ add_task(function* testDefaultDetails() {
             browser.test.sendMessage("ready");
           });
         });
-      }
+      },
+
+      files: {
+        "foo/bar.png": imageBuffer,
+        "baz/quux.png": imageBuffer,
+      },
     });
 
     yield Promise.all([extension.startup(), extension.awaitMessage("ready")]);
@@ -350,12 +421,12 @@ add_task(function* testDefaultDetails() {
     let pageActionId = makeWidgetId(extension.id) + "-page-action";
 
     let browserActionButton = document.getElementById(browserActionId);
-    let image = browserActionButton.getAttribute("image");
+    let image = getListStyleImage(browserActionButton);
 
     ok(expectedURL.test(image), `browser action image ${image} matches ${expectedURL}`);
 
     let pageActionImage = document.getElementById(pageActionId);
-    image = pageActionImage.src;
+    image = getListStyleImage(pageActionImage);
 
     ok(expectedURL.test(image), `page action image ${image} matches ${expectedURL}`);
 

browser/components/extensions/test/browser/browser_ext_pageAction_context.js

@@ -98,7 +98,7 @@ function* runTests(options) {
     } else {
       ok(image, "image exists");
 
-      is(image.src, details.icon, "icon URL is correct");
+      is(getListStyleImage(image), details.icon, "icon URL is correct");
 
       let title = details.title || options.manifest.name;
       is(image.getAttribute("tooltiptext"), title, "image title is correct");
@@ -177,6 +177,10 @@ add_task(function* testTabSwitchContext() {
           "description": "Title",
         },
       },
+
+      "default.png": imageBuffer,
+      "1.png": imageBuffer,
+      "2.png": imageBuffer,
     },
 
     getTests(tabs) {
@@ -309,6 +313,10 @@ add_task(function* testDefaultTitle() {
       "permissions": ["tabs"],
     },
 
+    files: {
+      "icon.png": imageBuffer,
+    },
+
     getTests(tabs) {
       let details = [
         {"title": "Foo Extension",

browser/components/extensions/test/browser/head.js

@@ -10,6 +10,7 @@
  *          promisePopupShown promisePopupHidden
  *          openContextMenu closeContextMenu
  *          openExtensionContextMenu closeExtensionContextMenu
+ *          imageBuffer getListStyleImage
  */
 
 var {AppConstants} = Cu.import("resource://gre/modules/AppConstants.jsm");
@@ -47,6 +48,17 @@ var focusWindow = Task.async(function* focusWindow(win) {
   yield promise;
 });
 
+let img = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQImWNgYGBgAAAABQABh6FO1AAAAABJRU5ErkJggg==";
+var imageBuffer = Uint8Array.from(atob(img), byte => byte.charCodeAt(0)).buffer;
+
+function getListStyleImage(button) {
+  let style = button.ownerDocument.defaultView.getComputedStyle(button);
+
+  let match = /^url\("(.*)"\)$/.exec(style.listStyleImage);
+
+  return match && match[1];
+}
+
 function promisePopupShown(popup) {
   return new Promise(resolve => {
     if (popup.state == "open") {
@@ -84,7 +96,7 @@ function getBrowserActionPopup(extension, win = window) {
   return win.PanelUI.panel;
 }
 
-var clickBrowserAction = Task.async(function* (extension, win = window) {
+var showBrowserAction = Task.async(function* (extension, win = window) {
   let group = getBrowserActionWidget(extension);
   let widget = group.forWindow(win);
 
@@ -93,6 +105,12 @@ var clickBrowserAction = Task.async(function* (extension, win = window) {
   } else if (group.areaType == CustomizableUI.TYPE_MENU_PANEL) {
     yield win.PanelUI.show();
   }
+});
+
+var clickBrowserAction = Task.async(function* (extension, win = window) {
+  yield showBrowserAction(extension, win);
+
+  let widget = getBrowserActionWidget(extension).forWindow(win);
 
   EventUtils.synthesizeMouseAtCenter(widget.node, {}, win);
 });

mobile/android/components/extensions/ext-pageAction.js

@@ -64,7 +64,8 @@ PageAction.prototype = {
 
     this.shouldShow = true;
 
-    let {icon} = IconDetails.getURL(this.icons, context.contentWindow, this.extension, 18);
+    let {icon} = IconDetails.getPreferredIcon(this.icons, this.extension,
+                                              18 * context.contentWindow.devicePixelRatio);
 
     let browserWindow = Services.wm.getMostRecentWindow("navigator:browser");
     return IconDetails.convertImageURLToDataURL(icon, context, browserWindow).then(dataURI => {

toolkit/components/extensions/ExtensionUtils.jsm

@@ -474,11 +474,9 @@ let IconDetails = {
 
   // Returns the appropriate icon URL for the given icons object and the
   // screen resolution of the given window.
-  getURL(icons, window, extension, size = 16) {
+  getPreferredIcon(icons, extension = null, size = 16) {
     const DEFAULT = "chrome://browser/content/extension.svg";
 
-    size *= window.devicePixelRatio;
-
     let bestSize = null;
     if (icons[size]) {
       bestSize = size;