Skip to content

Loading…

Fix for #1017 (and #1001) #1021

Merged
merged 2 commits into from

3 participants

@AlexVallat
Collaborator

Using .loadContext.DOMWindowID as an ID instead of tab.id resolves issues with new tabs in Firefox being seen as behind the scenes. Not re-binding to about:blank during load resolves further issues with opening new tabs from 3rd party intents

@AlexVallat
Collaborator

Also tested to fix #1019

@gorhill gorhill merged commit 38d308f into chrisaljoudi:master
@Deathamns

loadContext.DOMWindowID probably won't work with e10s, so this may be just a temporary fix.

@AlexVallat
Collaborator

I can't find any documentation that states definitively what is and is not accessible to the content or chrome processes without CPOW. I can say that when I tried accessing it through the console in Nightly with e10s turned on it never gave me a warning, but that's the best I can do until e10s is available on Fennec.

@Deathamns

Neither I got a warning, I got undefined instead. However, I tried on the desktop browser, but since those part should be the same, I assumed that this will happen in Fennec too (that's why I said "probably" and "may").

@AlexVallat
Collaborator

I've got another idea I can take a look at when I next get some time - it's looking like Firefox may not actually provide us with any usable ID, but perhaps we don't need it to. I was thinking that if we set up a weakmap of browser to an incrementing ID we assign ourselves, it can be looked up by just iterating over open tabs and looking up the ID we assigned them from the map. The access pattern is quite a lot of looking up ID from browser, and very little looking up browser from ID, so it shouldn't be too much of a hit.

@Deathamns

Wasn't the problem with Fennec, that it made the main_frame request before the tab opened? Because you need the tab to tell if a request is made in a tab, the browser may be ready, but behind-the-scene requests have browser objects too, but they don't have a tab.

@AlexVallat
Collaborator

Yeah, when the new tab is opened the browser is available but the tab is not yet. So any ID that hangs off the tab object is unusable.

@Deathamns

I stand corrected, behind-the-scene requests don't have a browser object (as far as I can tell), so this may work.

@Deathamns

I've made some tests, and it seems it will be viable.

@Deathamns

While I played with it, I kind of implemented it, so you can test it on this branch.

@AlexVallat
Collaborator

Great, that's just what I had in mind. I've tested it on Fennec and it appears to be working fine - I'll keep running with it for a while and see if I run into any problems, but otherwise I like this as a solution.

@dungsaga dungsaga pushed a commit that referenced this pull request
@gorhill gorhill #1021: more `uabInject` filters e1885e2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 13 additions and 15 deletions.
  1. +13 −15 platform/firefox/vapi-background.js
View
28 platform/firefox/vapi-background.js
@@ -308,7 +308,8 @@ var tabWatcher = {
onTabSelect: function({target}) {
// target is tab in Firefox, browser in Fennec
- var URI = (target.linkedBrowser || target).currentURI;
+ var browser = (target.linkedBrowser || target);
+ var URI = browser.currentURI;
var aboutPath = URI.schemeIs('about') && URI.path;
var tabId = vAPI.tabs.getTabId(target);
@@ -317,11 +318,13 @@ var tabWatcher = {
return;
}
- vAPI.tabs.onNavigation({
- frameId: 0,
- tabId: tabId,
- url: URI.asciiSpec
- });
+ if ( browser.webNavigation.busyFlags === 0 /*BUSY_FLAGS_NONE*/ ) {
+ vAPI.tabs.onNavigation({
+ frameId: 0,
+ tabId: tabId,
+ url: URI.asciiSpec
+ });
+ }
},
onLocationChange: function(browser, webProgress, request, location, flags) {
@@ -452,15 +455,9 @@ vAPI.tabs.getTabId = function(target) {
if ( vAPI.fennec ) {
if ( target.browser ) {
// target is a tab
- return target.id;
- }
-
- for ( var win of this.getWindows() ) {
- var tab = win.BrowserApp.getTabForBrowser(target);
- if ( tab && tab.id !== undefined ) {
- return tab.id;
- }
+ return target.browser.loadContext.DOMWindowID;
}
+ return target.loadContext.DOMWindowID;
return -1;
}
@@ -513,7 +510,8 @@ vAPI.tabs.getTabsForIds = function(tabIds, tabBrowser) {
if ( vAPI.fennec ) {
for ( tabId of tabIds ) {
- var tab = tabBrowser.getTabForId(tabId);
+
+ var tab = tabBrowser.tabs.find(tab=>tab.browser.loadContext.DOMWindowID === Number(tabId));
if ( tab ) {
tabs.push(tab);
}
Something went wrong with that request. Please try again.