Closed Bug 1347983 Opened 8 years ago Closed 8 years ago

Forms submitted from a large-allocation page discard post data

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox52 --- unaffected
firefox53 + fixed
firefox54 + fixed
firefox55 + fixed

People

(Reporter: nika, Assigned: nika)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 1 obsolete file)

Forms which are submitted from a large-allocation page currently discard their post data., This is because the function `nsIWebBrowserChrome3::ShouldLoadURI` is unaware of whether or not the requested load has any associated POST data. This adds that knowledge, and a test to ensure that these requests are handled correctly by large-allocation pages.
MozReview-Commit-ID: 7SEdTJN9Xd2
Attachment #8848137 - Flags: review?(bugs)
Comment on attachment 8848137 [details] [diff] [review] Part 1: Make forms submitted from a large-allocation page not leave the large-allocation process >+ if (!E10SUtils.shouldLoadURI(aDocShell, aURI, aReferrer, aHasPostData)) { >+ // XXX: Do we want to complain if we have post data but are still >+ // redirecting the load? Perhaps a telemetry probe? Theoretically we >+ // shouldn't do this, as it throws out data. ok, the comment isn't about this bug. Please file another bug and add the bug number somewhere here.
Attachment #8848137 - Flags: review?(bugs) → review+
Attachment #8848138 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #3) > Comment on attachment 8848137 [details] [diff] [review] > Part 1: Make forms submitted from a large-allocation page not leave the > large-allocation process > > >+ if (!E10SUtils.shouldLoadURI(aDocShell, aURI, aReferrer, aHasPostData)) { > >+ // XXX: Do we want to complain if we have post data but are still > >+ // redirecting the load? Perhaps a telemetry probe? Theoretically we > >+ // shouldn't do this, as it throws out data. > ok, the comment isn't about this bug. > Please file another bug and add the bug number somewhere here. Filed bug 1348018.
Attachment #8848137 - Attachment is obsolete: true
Pushed by michael@thelayzells.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3b2c7853ae71 Part 1: Make forms submitted from a large-allocation page not leave the large-allocation process, r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/38ecd019eceb Part 2: Add a test to ensure that forms submitted from a large-allocation page behave correctly, r=smaug
Comment on attachment 8848179 [details] [diff] [review] Part 1: Make forms submitted from a large-allocation page not leave the large-allocation process Approval Request Comment [Feature/Bug causing the regression]: bug 1331083 [User impact if declined]: POST forms submitted on pages with the Large-Allocation header will not work correctly on 32-bit windows builds of Firefox. [Is this code covered by automated tests?]: Yes [Has the fix been verified in Nightly?]: Just landed on inbound [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: Not very [Why is the change risky/not risky?]: Makes change to feature which will affect a very small number of sites (only those which opt into the large allocation header). Actual changes made by the patch are fairly limited in scope. [String changes made/needed]: None
Attachment #8848179 - Flags: approval-mozilla-beta?
Attachment #8848179 - Flags: approval-mozilla-aurora?
Attachment #8848138 - Flags: approval-mozilla-beta?
Attachment #8848138 - Flags: approval-mozilla-aurora?
I rebased the patches onto beta (apply these patches to beta instead of the original ones - they will apply cleanly) MozReview-Commit-ID: 7SEdTJN9Xd2
Tracking 53/54/55+ for this regression, for the reasons outlined in Comment 7.
Comment on attachment 8848179 [details] [diff] [review] Part 1: Make forms submitted from a large-allocation page not leave the large-allocation process Fix a regression that POST forms submitted on pages with the Large-Allocation header won't work correctly and include tests. Aurora54+ & Beta53+.
Attachment #8848179 - Flags: approval-mozilla-beta?
Attachment #8848179 - Flags: approval-mozilla-beta+
Attachment #8848179 - Flags: approval-mozilla-aurora?
Attachment #8848179 - Flags: approval-mozilla-aurora+
Attachment #8848138 - Flags: approval-mozilla-beta?
Attachment #8848138 - Flags: approval-mozilla-beta+
Attachment #8848138 - Flags: approval-mozilla-aurora?
Attachment #8848138 - Flags: approval-mozilla-aurora+
has problems to apply to beta grafting 406008:93fee9a2f837 "Bug 1347983 - Part 1: Make forms submitted from a large-allocation page not leave the large-allocation process, r=smaug a=gchang" merging browser/base/content/browser.js merging browser/modules/E10SUtils.jsm merging docshell/base/nsDocShell.cpp warning: conflicts while merging browser/modules/E10SUtils.jsm! (edit, then use 'hg resolve --mark') warning: conflicts while merging docshell/base/nsDocShell.cpp! (edit, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use 'hg resolve' and 'hg graft --continue') could you take a look, thanks!
Flags: needinfo?(michael)
never mind, took the beta patches
Flags: needinfo?(michael)
Setting qe-verify- based on Michael's assessment on manual testing needs (see Comment 7) and the fact that this fix has automated coverage.
Flags: qe-verify-
Depends on: 1351702
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: