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)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla55
People
(Reporter: nika, Assigned: nika)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 1 obsolete file)
(deleted),
patch
|
smaug
:
review+
gchang
:
approval-mozilla-aurora+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
gchang
:
approval-mozilla-aurora+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
BETA Part 2: Add a test to ensure that forms submitted from a large-allocation page behave correctly
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•8 years ago
|
status-firefox52:
--- → unaffected
status-firefox53:
--- → affected
status-firefox54:
--- → affected
status-firefox55:
--- → affected
tracking-firefox53:
--- → ?
tracking-firefox54:
--- → ?
tracking-firefox55:
--- → ?
Assignee | ||
Comment 1•8 years ago
|
||
MozReview-Commit-ID: 7SEdTJN9Xd2
Attachment #8848137 -
Flags: review?(bugs)
Assignee | ||
Comment 2•8 years ago
|
||
MozReview-Commit-ID: Af44H11AFMf
Attachment #8848138 -
Flags: review?(bugs)
Comment 3•8 years ago
|
||
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+
Updated•8 years ago
|
Attachment #8848138 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 4•8 years ago
|
||
(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.
Assignee | ||
Comment 5•8 years ago
|
||
MozReview-Commit-ID: 7SEdTJN9Xd2
Assignee | ||
Updated•8 years ago
|
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
Assignee | ||
Comment 7•8 years ago
|
||
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?
Assignee | ||
Updated•8 years ago
|
Attachment #8848138 -
Flags: approval-mozilla-beta?
Attachment #8848138 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 8•8 years ago
|
||
I rebased the patches onto beta (apply these patches to beta instead of the original ones - they will apply cleanly)
MozReview-Commit-ID: 7SEdTJN9Xd2
Assignee | ||
Comment 9•8 years ago
|
||
MozReview-Commit-ID: Af44H11AFMf
Comment 10•8 years ago
|
||
Tracking 53/54/55+ for this regression, for the reasons outlined in Comment 7.
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3b2c7853ae71
https://hg.mozilla.org/mozilla-central/rev/38ecd019eceb
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 12•8 years ago
|
||
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+
Updated•8 years ago
|
Attachment #8848138 -
Flags: approval-mozilla-beta?
Attachment #8848138 -
Flags: approval-mozilla-beta+
Attachment #8848138 -
Flags: approval-mozilla-aurora?
Attachment #8848138 -
Flags: approval-mozilla-aurora+
Comment 13•8 years ago
|
||
bugherder uplift |
Comment 14•8 years ago
|
||
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)
Comment 16•8 years ago
|
||
bugherder uplift |
Comment 17•8 years ago
|
||
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-
Updated•7 years ago
|
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•