Closed Bug 1351702 Opened 5 years ago Closed 5 years ago
.js doesn't implement ns IWeb Browser Chrome3::should Load URI correctly
The patch in bug 1347983 didn't change shouldLoadURI in tab-content.js. So, aTriggeringPrincipal is set to a bool and even if we try and redirect the load it fails trying to serialise the principal. [Tracking Requested - why for this release]: Bug 1347983 has been uplifted to Fx53.
Comment on attachment 8852549 [details] [diff] [review] Correct shouldLoadURI in tab-content.js to add aHasPostData argument Review of attachment 8852549 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for catching this!
Attachment #8852549 - Flags: review?(michael) → review+
Looks like pulsebot is out of action: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/09c3073768242d0917af5b633cf0c94fd37571d3
Comment on attachment 8852549 [details] [diff] [review] Correct shouldLoadURI in tab-content.js to add aHasPostData argument Approval Request Comment [Feature/Bug causing the regression]: Bug 1347983 - which has been uplifted to Fx53. [User impact if declined]: I don't have an actual test case showing an issue (in Fx53) that this causes, but as it stands the call to shouldLoadURI from nsDocShell when doing a POST would always fail, even if we are in a situation where the load should occur in another process. For non-POST calls the security principal will not be serialised. This definitely affects the file content process in Fx55. [Is this code covered by automated tests?]: The code is exercised by tests, but clearly not this particular scenario. [Has the fix been verified in Nightly?]: Problem was found while working on bug 1351358 and fix verified on local build. [Needs manual test from QE? If yes, steps to reproduce]: Yes, I'll upload a test case for the file content process with STR which can be used in Nightly. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Very small change, which corrects a definite error in the code. [String changes made/needed]: No.
I am a little worried we are still cleaning up issues from bug 1331083.
Comment on attachment 8852549 [details] [diff] [review] Correct shouldLoadURI in tab-content.js to add aHasPostData argument Fix for a regression in 53, let's uplift to beta.
Flagging this for manual testing, instructions (for Nightly) available in Comment 6. Bob, would it be possible for you to provide a similar testcase for Beta 53 as well? Or perhaps there's a workaround Manual QA could use to adapt the testcase you created for Nightly?
(In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #11) > Flagging this for manual testing, instructions (for Nightly) available in > Comment 6. > > Bob, would it be possible for you to provide a similar testcase for Beta 53 > as well? Or perhaps there's a workaround Manual QA could use to adapt the > testcase you created for Nightly? They could flip the browser.tabs.remote.separateFileUriProcess pref to true, to see the same issue with the file content process in aurora and beta. This at least demonstrates the issue and fix, but the file content process is actually only enabled on Nightly. I don't have a different scenario that demonstrates the problem without doing that, if I think of something I'll let you know.
Hi, I have reproduced this issue and I can confirm that it is VERIFIED FIXED in: - Nightly 55.0a1 (id: 20170406030206) - Aurora 54.0a2 (id: 20170406004017) - Beta 53.0b9 I used Windows 10 x64 for testing this. User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0
You need to log in before you can comment on or make changes to this bug.