Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: ext/background.js

Issue 29995555: Issue 7253 - Pre-render icons for badge on Chromium (Closed) Base URL: https://hg.adblockplus.org/adblockpluschrome/
Patch Set: Restrict to Chromium Created Feb. 2, 2019, 11:51 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | lib/icon.js » ('j') | lib/icon.js » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ext/background.js
===================================================================
--- a/ext/background.js
+++ b/ext/background.js
@@ -322,28 +322,43 @@
_applyChanges()
{
return Promise.all(Object.keys(this._changes).map(change =>
{
// Firefox for Android displays the browser action not as an icon but
// as a menu item. There is no icon, but such an option may be added
// in the future.
// https://bugzilla.mozilla.org/show_bug.cgi?id=1331746
- if (change == "iconPath" && "setIcon" in browser.browserAction)
+ if (change == "icon" && "setIcon" in browser.browserAction)
{
let 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")
+ 16: this._changes.icon.path.replace("$size", "16"),
+ 19: this._changes.icon.path.replace("$size", "19"),
+ 20: this._changes.icon.path.replace("$size", "20"),
+ 32: this._changes.icon.path.replace("$size", "32"),
+ 38: this._changes.icon.path.replace("$size", "38"),
+ 40: this._changes.icon.path.replace("$size", "40")
Sebastian Noack 2019/02/03 06:43:18 This should go after trying to use imageData. If w
Manish Jethani 2019/02/03 07:26:48 Done.
};
try
{
+ // Try ImageData if available.
+ if (this._changes.icon.imageData)
+ {
+ try
Sebastian Noack 2019/02/03 06:43:18 Why do we need try...catch here? Can't we just che
Manish Jethani 2019/02/03 07:26:48 Yes, makes sense. Done.
+ {
+ return browser.browserAction.setIcon({
+ tabId: this._tabId,
+ imageData: this._changes.icon.imageData
+ });
+ }
+ catch (e)
+ {
+ }
+ }
+
return browser.browserAction.setIcon({tabId: this._tabId, path});
}
catch (e)
{
// Edge throws if passed icon sizes different than 19,20,38,40px.
delete path[16];
delete path[32];
return browser.browserAction.setIcon({tabId: this._tabId, path});
@@ -394,19 +409,19 @@
{
// If the tab is prerendered, browser.browserAction.set* fails
// and we have to delay our changes until the currently visible tab
// is replaced with the prerendered tab.
browser.tabs.onReplaced.addListener(onReplaced);
});
}
},
- setIcon(path)
+ setIcon(path, imageData = null)
{
- this._addChange("iconPath", path);
+ this._addChange("icon", {path, imageData});
},
setBadge(badge)
{
if (!badge)
{
this._addChange("badgeText", "");
}
else
« no previous file with comments | « no previous file | lib/icon.js » ('j') | lib/icon.js » ('J')

Powered by Google App Engine
This is Rietveld