Issue 4699 - Edge does not support 16px and 32px icons

chrome/ext/background.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 176 matching lines @@
   });
 
   chrome.tabs.onRemoved.addListener(forgetTab);
 
   chrome.tabs.onActivated.addListener(function(details)
   {
     ext.pages.onActivated._dispatch(new Page({id: details.tabId}));
   });
 
 
+  ext.iconSizes = [16, 19, 20, 32, 38, 40];
+
   /* Browser actions */
 
   var BrowserAction = function(tabId)
   {
     this._tabId = tabId;
     this._changes = null;
   };
   BrowserAction.prototype = {
-    _legacySetIcon: function(details)
-    {
-      var legacyDetails = {};
-      for (var key in details)
-      {
-        var value = details[key];
-        if (typeof value == "object")
-          value = {19: value[19], 38: value[38]};
-        legacyDetails[key] = value;
-      }
-      chrome.browserAction.setIcon(legacyDetails);
-    },
     _safeSetIcon: function(details)
     {
       try
       {
         chrome.browserAction.setIcon(details);
       }
       catch (e)
       {
-        // Older versions of Chrome do not allow any sizes other than 19 and 38
-        // to be present, but newer versions of Chrome (and Edge) prefer
-        // different sizes.
-        this._safeSetIcon = this._legacySetIcon;
-        this._legacySetIcon(details);

[Oleksandr, 2016/12/05 05:29:59]

In this case, the `this` pointer is refreshed for every icon change, so
reloading the function does not make much sense. It gets reset to default on the
next icon change anyway.

[kzar, 2016/12/05 16:17:56]

Well a BrowserAction is created for each page, so this logic is repeated once
for each tab but not every time the icon is changed. (Possibly significant on
slower computers since the animations involve changing the icon over and over
again in short succession.)

(I suppose this could be improved by assigning
BrowserAction.prototype._safeSetIcon here.)

+        // Try applying the icon the Edge way
+        ext.iconSizes = [19, 20, 38, 40];                  
+        try
+        {
+          this.applyIcon();
+        }
+        catch(e)
+        {
+          // Just stick to the bare minimum. Older versions of Chrome do not
+          // allow any sizes other than 19 and 38 to be present
+          ext.iconSizes = [19, 38];          
+          applyIcon();
+        }
       }
     },
+
+    applyIcon: function()
+    {
+      var builtPath = {};
+      currentIconPath = this._changes.iconPath;
+      ext.iconSizes.map(function(iconSize)
+      {
+        builtPath[iconSize] = currentIconPath.replace("$size", iconSize); 
+      });
+
+      this._safeSetIcon({
+        tabId: this._tabId,
+        path: builtPath
+      });
+    },
+
     _applyChanges: function()
     {
       if ("iconPath" in this._changes)
-      {
-        this._safeSetIcon({
-          tabId: this._tabId,
-          path: {
-            16: this._changes.iconPath.replace("$size", "16"),
-            19: this._changes.iconPath.replace("$size", "19"),
-            20: this._changes.iconPath.replace("$size", "20"),
-            32: this._changes.iconPath.replace("$size", "32"),
-            38: this._changes.iconPath.replace("$size", "38"),
-            40: this._changes.iconPath.replace("$size", "40")
-          }
-        });
-      }
+        this.applyIcon();
 
       if ("badgeText" in this._changes)
       {
         chrome.browserAction.setBadgeText({
           tabId: this._tabId,
           text: this._changes.badgeText
         });
       }
 
       if ("badgeColor" in this._changes)
@@ skipping 389 matching lines @@
             afterTabLoaded(callback)(createdWindow.tabs[0]);
         });
       }
       else
       {
         ext.pages.open(createData.url, callback);
       }
     }
   };
 })();