Bug 1397196 - Fix pageAction icon loading when an extension has a cached browserAction theme-based icon.
MozReview-Commit-ID: Lmi5pLerzul
--- a/browser/components/extensions/ext-browserAction.js
+++ b/browser/components/extensions/ext-browserAction.js
@@ -100,16 +100,17 @@ this.browserAction = class extends Exten
}
browserActionMap.set(extension, this);
this.defaults.icon = await StartupCache.get(
extension, ["browserAction", "default_icon"],
() => IconDetails.normalize({
path: options.default_icon,
+ iconType: "browserAction",
themeIcons: options.theme_icons,
}, extension));
this.iconData.set(
this.defaults.icon,
await StartupCache.get(
extension, ["browserAction", "default_icon_data"],
() => this.getIconData(this.defaults.icon)));
@@ -617,16 +618,18 @@ this.browserAction = class extends Exten
let title = browserAction.getProperty(tab, "title");
return Promise.resolve(title);
},
setIcon: function(details) {
let tab = getTab(details.tabId);
+ details.iconType = "browserAction";
+
let icon = IconDetails.normalize(details, extension, context);
browserAction.setProperty(tab, "icon", icon);
},
setBadgeText: function(details) {
let tab = getTab(details.tabId);
browserAction.setProperty(tab, "badgeText", details.text);
--- a/browser/components/extensions/ext-pageAction.js
+++ b/browser/components/extensions/ext-pageAction.js
@@ -143,22 +143,21 @@ this.pageAction = class extends Extensio
button.setAttribute("style", style);
}
button.hidden = !tabData.show;
});
}
getIconData(icons) {
- // 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(icons, this.extension, size).icon);
+ let getIcon = size => {
+ let {icon} = IconDetails.getPreferredIcon(icons, this.extension, size);
+ // TODO: implement theme based icon for pageAction (Bug 1398156)
+ return IconDetails.escapeUrl(icon);
+ };
let style = `
--webextension-urlbar-image: url("${getIcon(16)}");
--webextension-urlbar-image-2x: url("${getIcon(32)}");
`;
return {style};
}
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js
@@ -292,8 +292,80 @@ add_task(async function testDetailsObjec
if (!test.menuResolutions) {
continue;
}
}
await extension.unload();
});
+
+// NOTE: The current goal of this test is ensuring that Bug 1397196 has been fixed,
+// and so this test extension manifest has a browser action which specify
+// a theme based icon and a pageAction, both the pageAction and the browserAction
+// have a common default_icon.
+//
+// Once Bug 1398156 will be fixed, this test should be converted into testing that
+// the browserAction and pageAction themed icons (as well as any other cached icon,
+// e.g. the sidebar and devtools panel icons) can be specified in the same extension
+// and do not conflict with each other.
+//
+// This test currently fails without the related fix, but only if the browserAction
+// API has been already loaded before the pageAction, otherwise the icons will be cached
+// in the opposite order and the test is not able to reproduce the issue anymore.
+add_task(async function testPageActionIconLoadingOnBrowserActionThemedIcon() {
+ async function background() {
+ const tabs = await browser.tabs.query({active: true});
+ await browser.pageAction.show(tabs[0].id);
+
+ browser.test.sendMessage("ready");
+ }
+
+ const extension = ExtensionTestUtils.loadExtension({
+ background,
+ manifest: {
+ "name": "Foo Extension",
+
+ "browser_action": {
+ "default_icon": "common_cached_icon.png",
+ "default_popup": "default_popup.html",
+ "default_title": "BrowserAction title",
+ "theme_icons": [
+ {
+ "dark": "1.png",
+ "light": "2.png",
+ "size": 16
+ }
+ ],
+ },
+
+ "page_action": {
+ "default_icon": "common_cached_icon.png",
+ "default_popup": "default_popup.html",
+ "default_title": "PageAction title",
+ },
+
+ "permissions": ["tabs"],
+ },
+
+ "files": {
+ "common_cached_icon.png": imageBuffer,
+ "1.png": imageBuffer,
+ "2.png": imageBuffer,
+ "default_popup.html": "<!DOCTYPE html><html><body>popup</body></html>",
+ },
+ });
+
+ await extension.startup();
+
+ await extension.awaitMessage("ready");
+
+ await promiseAnimationFrame();
+
+ let pageActionId = `${makeWidgetId(extension.id)}-page-action`;
+ let pageActionImage = document.getElementById(pageActionId);
+
+ const iconURL = new URL(getListStyleImage(pageActionImage));
+
+ is(iconURL.pathname, "/common_cached_icon.png", "Got the expected pageAction icon url");
+
+ await extension.unload();
+});
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -1219,18 +1219,20 @@ function extensionNameFromURI(uri) {
}
}
return GlobalManager.getExtension(id).name;
}
// Manages icon details for toolbar buttons in the |pageAction| and
// |browserAction| APIs.
let IconDetails = {
- // WeakMap<Extension -> Map<url-string -> object>>
- iconCache: new DefaultWeakMap(() => new DefaultMap(() => new Map())),
+ // WeakMap<Extension -> Map<url-string -> Map<iconType-string -> object>>>
+ iconCache: new DefaultWeakMap(() => {
+ return new DefaultMap(() => new DefaultMap(() => new Map()));
+ }),
// Normalizes the various acceptable input formats into an object
// with icon size as key and icon URL as value.
//
// If a context is specified (function is called from an extension):
// Throws an error if an invalid icon size was provided or the
// extension is not allowed to load the specified resources.
//
@@ -1241,17 +1243,18 @@ let IconDetails = {
// Pick a cache key for the icon paths. If the path is a string,
// use it directly. Otherwise, stringify the path object.
let key = details.path;
if (typeof key !== "string") {
key = uneval(key);
}
let icons = this.iconCache.get(extension)
- .get(context && context.uri.spec);
+ .get(context && context.uri.spec)
+ .get(details.iconType);
let icon = icons.get(key);
if (!icon) {
icon = this._normalize(details, extension, context);
icons.set(key, icon);
}
return icon;
}