Bug 1147911 changed Firefox so that top level loads of file:// URI pages are done in separate file content processes. If the file:// page frames http pages they are also loaded in the file content process, due to current cross-process referencing limitations. However if a file:// page causes a top level http load (via link or JS) then that gets loaded in a normal content process. Again because of cross process referencing limitations, in this case the window.opener is not set and window.open returns null. There is some concern that this might cause compatibility issues, particularly with corporate users, so we want to add a pref that allows http content linked from file pages to load in the same process even when it is a top level load.

(In reply to Bob Owen (:bobowen) from comment #0) > There is some concern that this might cause compatibility issues, > particularly with corporate users, so we want to add a pref that allows http > content linked from file pages to load in the same process even when it is a > top level load. What does this gain us over the extant pref to just disable the separate file process, and is that really sufficient gain to justify the additional cost + risk in code complexity, testing, etc.?

(In reply to :Gijs from comment #1) > (In reply to Bob Owen (:bobowen) from comment #0) > > There is some concern that this might cause compatibility issues, > > particularly with corporate users, so we want to add a pref that allows http > > content linked from file pages to load in the same process even when it is a > > top level load. > > What does this gain us over the extant pref to just disable the separate > file process, and is that really sufficient gain to justify the additional > cost + risk in code complexity, testing, etc.? If we disable the file content process then we can't remove read access from the normal content process. The change is very simple, I already have a patch. I need to write a test, should be ready tomorrow.

Try push with the fix from bug 1345807 included (I've fixed the lint issues locally): https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c4c17a736362f90ea8287fb6ed43e4b9e553d0d Another try push with the new pref flipped on: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3de5ee9d7fe380bc4e28f947049fe3142c65ccd9

Comment on attachment 8845401 [details] [diff] [review] Part 2: Check that related web content loads in file content process when pref flipped Review of attachment 8845401 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/test/tabs/browser.ini @@ +8,5 @@ > skip-if = !e10s # Tab spinner is e10s only. > [browser_tabSwitchPrintPreview.js] > skip-if = os == 'mac' > [browser_navigatePinnedTab.js] > +[browser_new_web_tab_in_file_process_pref.js] I suppose this will work in non-e10s, but there's little point running it there, so you could use skip-if = !e10s ::: browser/base/content/test/tabs/browser_new_web_tab_in_file_process_pref.js @@ +11,5 @@ > + dir.append(TEST_FILE); > + const uriString = Services.io.newFileURI(dir).spec; > + let fileTab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, uriString); > + registerCleanupFunction(function* () { > + yield BrowserTestUtils.removeTab(fileTab); Here too you could use withNewTab instead for slightly simpler code. :-) @@ +18,5 @@ > + // Set pref to allow linked web content in file URI process. > + Services.prefs.setBoolPref("browser.tabs.remote.allowLinkedWebInFileUriProcess", true); > + registerCleanupFunction(function() { > + Services.prefs.clearUserPref("browser.tabs.remote.allowLinkedWebInFileUriProcess"); > + }); Nit: use: yield SpecialPowers.pushPrefEnv({set: [["browser.tabs.remote.allowLinkedWebInFileUriProcess", true]]}); I actually expect that we should probably enable the separate file process this way, too, given that that's not guaranteed to be riding the trains yet.

(In reply to :Gijs from comment #6) > Comment on attachment 8845401 [details] [diff] [review] > Part 2: Check that related web content loads in file content process when > pref flipped > > +[browser_new_web_tab_in_file_process_pref.js] > > I suppose this will work in non-e10s, but there's little point running it > there, so you could use skip-if = !e10s Yes, changed locally, thanks. > Nit: use: > > yield SpecialPowers.pushPrefEnv({set: > [["browser.tabs.remote.allowLinkedWebInFileUriProcess", true]]}); > > I actually expect that we should probably enable the separate file process > this way, too, given that that's not guaranteed to be riding the trains yet. Yes again it only really makes sense with that pref set.

Pushed by bobowencode@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0ccde656af1c Part 1: Add pref to allow linked web content to load in file content process. r=Gijs https://hg.mozilla.org/integration/mozilla-inbound/rev/0055f45d1315 Part 2: Check that related web content loads in file content process when pref flipped. r=Gijs

Pushed by bobowencode@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c28111d13613 part 3: Follow-up to fix no-shadow lint issue. r=me

https://hg.mozilla.org/mozilla-central/rev/0ccde656af1c https://hg.mozilla.org/mozilla-central/rev/0055f45d1315 https://hg.mozilla.org/mozilla-central/rev/c28111d13613

Reproduced the initial issue using Nightly 54.0a1 (Build ID: 20170301030202) on Windows 10 x64. Verified as fixed using the latest Nightly 55.0a1 (Build ID: 20170523030206) on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12.