Bug 1413648 fix default extension icon use, r?aswan draft
authorShane Caraveo <scaraveo@mozilla.com>
Thu, 02 Nov 2017 15:49:38 -0700
changeset 692282 820311d41361f0d75af3423f0278273d3e12676d
parent 690034 cd7217cf05a2332a8fd7b498767a07b2c31ea657
child 738730 e07fe62444a071cdec231ee6a6806214d43e5bc2
push id87469
push usermixedpuppy@gmail.com
push dateThu, 02 Nov 2017 22:50:01 +0000
reviewersaswan
bugs1413648
milestone58.0a1
Bug 1413648 fix default extension icon use, r?aswan MozReview-Commit-ID: 3Dh2DjGIAgn
browser/components/extensions/ext-pageAction.js
browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js
--- a/browser/components/extensions/ext-pageAction.js
+++ b/browser/components/extensions/ext-pageAction.js
@@ -62,17 +62,17 @@ this.pageAction = class extends Extensio
     this.defaults.icon = await StartupCache.get(
       extension, ["pageAction", "default_icon"],
       () => IconDetails.normalize({path: options.default_icon}, extension));
 
     if (!this.browserPageAction) {
       this.browserPageAction = PageActions.addAction(new PageActions.Action({
         id: widgetId,
         title: this.defaults.title,
-        iconURL: this.defaults.icon,
+        iconURL: this.getIconData(this.defaults.icon),
         shownInUrlbar: true,
         disabled: true,
         onCommand: (event, buttonNode) => {
           this.handleClick(event.target.ownerGlobal);
         },
         onBeforePlacedInWindow: browserWindow => {
           if (this.extension.hasPermission("menus") ||
               this.extension.hasPermission("contextMenus")) {
@@ -132,24 +132,33 @@ this.pageAction = class extends Extensio
     this.browserPageAction.setTitle(title, window);
     this.browserPageAction.setTooltip(title, window);
     this.browserPageAction.setDisabled(!tabData.show, window);
 
     let iconURL;
     if (typeof(tabData.icon) == "string") {
       iconURL = IconDetails.escapeUrl(tabData.icon);
     } else {
-      iconURL = Object.entries(tabData.icon).reduce((memo, [size, url]) => {
-        memo[size] = IconDetails.escapeUrl(url);
-        return memo;
-      }, {});
+      iconURL = this.getIconData(tabData.icon);
     }
     this.browserPageAction.setIconURL(iconURL, window);
   }
 
+  getIconData(icons) {
+    let getIcon = size => {
+      let {icon} = IconDetails.getPreferredIcon(icons, this.extension, size);
+      // TODO: implement theme based icon for pageAction (Bug 1398156)
+      return IconDetails.escapeUrl(icon);
+    };
+    return {
+      "16": getIcon(16),
+      "32": getIcon(32),
+    };
+  }
+
   /**
    * Triggers this page action for the given window, with the same effects as
    * if it were clicked by a user.
    *
    * This has no effect if the page action is hidden for the selected tab.
    *
    * @param {Window} window
    */
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js
@@ -259,26 +259,30 @@ add_task(async function testDetailsObjec
   const RESOLUTION_PREF = "layout.css.devPixelsPerPx";
 
   await extension.startup();
 
   let pageActionId = BrowserPageActions.urlbarButtonNodeIDForActionID(makeWidgetId(extension.id));
   let browserActionWidget = getBrowserActionWidget(extension);
 
   let tests = await extension.awaitMessage("ready");
+
+  // The initial icon should be the default icon since no icon is in the manifest.
+  const DEFAULT_ICON = "chrome://browser/content/extension.svg";
+  let browserActionButton = browserActionWidget.forWindow(window).node;
+  let pageActionImage = document.getElementById(pageActionId);
+  is(getListStyleImage(browserActionButton), DEFAULT_ICON, `browser action has the correct default image`);
+  is(getListStyleImage(pageActionImage), DEFAULT_ICON, `page action has the correct default image`);
+
   for (let test of tests) {
     extension.sendMessage("setIcon", test);
     await extension.awaitMessage("iconSet");
 
     await promiseAnimationFrame();
 
-    let browserActionButton = browserActionWidget.forWindow(window).node;
-    let pageActionImage = document.getElementById(pageActionId);
-
-
     // Test icon sizes in the toolbar/urlbar.
     for (let resolution of Object.keys(test.resolutions)) {
       await 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`);