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

Unified Diff: ext/background.js

Issue 29527877: Issue 5582 - Workaround Edge bug in tabs.query (Closed)
Patch Set: Rebase to master tip Created Aug. 29, 2017, 11:48 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 | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ext/background.js
===================================================================
--- a/ext/background.js
+++ b/ext/background.js
@@ -732,52 +732,43 @@
});
}
}
- else
+ else if ("windows" in chrome)
{
// Edge does not yet support runtime.openOptionsPage (tested version 38)
- // nor does Firefox for Android before version 57,
Oleksandr 2017/08/29 11:49:58 Remove this line, since this is really an Edge spe
// and so this workaround needs to stay for now.
// We are not using extension.getURL to get the absolute path here
// because of the Edge issue:
// https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/10276332/
- if ("windows" in chrome)
- {
- chrome.windows.getLastFocused(win =>
- {
- let optionsUrl = "options.html";
- let queryInfo = {url: optionsUrl};
+ let optionsUrl = "options.html";
+ let fullOptionsUrl = ext.getURL(optionsUrl);
- if (win)
- queryInfo.windowId = win.id;
-
- chrome.tabs.query(queryInfo, tabs =>
- {
- if (tabs.length > 0)
- {
- let tab = tabs[0];
-
- if ("windows" in chrome)
- chrome.windows.update(tab.windowId, {focused: true});
+ chrome.tabs.query({}, tabs =>
+ {
+ // We find a tab ourselves because Edge has a bug when quering tabs
+ // with extension URL protocol:
+ // https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/8094141/
+ // https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/8604703/
+ let tab = tabs.find(element => element.url == fullOptionsUrl);
+ if (tab)
+ {
+ chrome.windows.update(tab.windowId, {focused: true});
+ chrome.tabs.update(tab.id, {active: true});
- chrome.tabs.update(tab.id, {active: true});
-
- if (callback)
- callback(new Page(tab));
- }
- else
- {
- ext.pages.open(optionsUrl, callback);
- }
- });
- });
- }
- else
- {
- // Firefox for Android does not support the windows API. Since there is
- // effectively only one window on the mobile browser, there's no need
- // to bring it into focus.
- ext.pages.open("options.html", callback);
- }
+ if (callback)
+ callback(new Page(tab));
+ }
+ else
+ {
+ ext.pages.open(optionsUrl, callback);
+ }
+ });
+ }
+ else
+ {
+ // Firefox for Android does not support the windows API. Since there is
Manish Jethani 2017/08/29 12:09:59 Maybe it's a good idea to update this comment here
+ // effectively only one window on the mobile browser, there's no need
+ // to bring it into focus.
+ ext.pages.open("options.html", callback);
}
};
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld