Bug 1402025 - Ensure form submission flush when unsetting relevant attributes
It seems that we were flushing any pending submission when changing the action or target attributes of a form, but not when unsetting those attributes.
MozReview-Commit-ID: E6aUnokg54k
--- a/dom/html/HTMLFormElement.cpp
+++ b/dom/html/HTMLFormElement.cpp
@@ -184,31 +184,28 @@ HTMLFormElement::GetElements(nsIDOMHTMLC
}
nsresult
HTMLFormElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
const nsAttrValueOrString* aValue, bool aNotify)
{
if (aNamespaceID == kNameSpaceID_None) {
if (aName == nsGkAtoms::action || aName == nsGkAtoms::target) {
- // This check is mostly to preserve previous behavior.
- if (aValue) {
- if (mPendingSubmission) {
- // aha, there is a pending submission that means we're in
- // the script and we need to flush it. let's tell it
- // that the event was ignored to force the flush.
- // the second argument is not playing a role at all.
- FlushPendingSubmission();
- }
- // Don't forget we've notified the password manager already if the
- // page sets the action/target in the during submit. (bug 343182)
- bool notifiedObservers = mNotifiedObservers;
- ForgetCurrentSubmission();
- mNotifiedObservers = notifiedObservers;
+ if (mPendingSubmission) {
+ // aha, there is a pending submission that means we're in
+ // the script and we need to flush it. let's tell it
+ // that the event was ignored to force the flush.
+ // the second argument is not playing a role at all.
+ FlushPendingSubmission();
}
+ // Don't forget we've notified the password manager already if the
+ // page sets the action/target in the during submit. (bug 343182)
+ bool notifiedObservers = mNotifiedObservers;
+ ForgetCurrentSubmission();
+ mNotifiedObservers = notifiedObservers;
}
}
return nsGenericHTMLElement::BeforeSetAttr(aNamespaceID, aName, aValue,
aNotify);
}
nsresult
--- a/dom/html/test/browser.ini
+++ b/dom/html/test/browser.ini
@@ -1,16 +1,18 @@
[DEFAULT]
support-files =
bug592641_img.jpg
dummy_page.html
file_bug649778.html
file_bug649778.html^headers^
file_fullscreen-api-keys.html
file_content_contextmenu.html
+ submission_flush.html
+ post_action_page.html
form_data_file.bin
form_data_file.txt
form_submit_server.sjs
head.js
[browser_bug592641.js]
[browser_bug649778.js]
skip-if = e10s # Bug 1271025
@@ -22,8 +24,9 @@ support-files =
file_bug1108547-3.html
[browser_content_contextmenu_userinput.js]
[browser_DOMDocElementInserted.js]
[browser_form_post_from_file_to_http.js]
[browser_fullscreen-api-keys.js]
tags = fullscreen
[browser_fullscreen-contextmenu-esc.js]
tags = fullscreen
+[browser_submission_flush.js]
new file mode 100644
--- /dev/null
+++ b/dom/html/test/browser_submission_flush.js
@@ -0,0 +1,85 @@
+"use strict";
+// Form submissions triggered by the Javascript 'submit' event listener are
+// deferred until the event listener finishes. However, changes to specific
+// attributes ("action" and "target" attributes) need to cause an immediate
+// flush of any pending submission to prevent the form submission from using the
+// wrong action or target. This test ensures that such flushes happen properly.
+
+const kTestPage = "http://example.org/browser/dom/html/test/submission_flush.html";
+// This is the page pointed to by the form action in the test HTML page.
+const kPostActionPage = "http://example.org/browser/dom/html/test/post_action_page.html";
+
+const kFormId = "test_form"
+const kFrameId = "test_frame"
+const kSubmitButtonId = "submit_button"
+
+// Take in a variety of actions (in the form of setting and unsetting form
+// attributes). Then submit the form in the submit event listener to cause a
+// deferred form submission. Then perform the test actions and ensure that the
+// form used the correct attribute values rather than the changed ones.
+async function runTest(aTestActions) {
+ let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, kTestPage);
+ registerCleanupFunction(() => BrowserTestUtils.removeTab(tab));
+
+ let frame_url = await ContentTask.spawn(gBrowser.selectedBrowser,
+ {kFormId, kFrameId, kSubmitButtonId, aTestActions},
+ async function({kFormId, kFrameId, kSubmitButtonId,
+ aTestActions}) {
+ let form = content.document.getElementById(kFormId);
+
+ form.addEventListener("submit", (event) => {
+ // Need to trigger the deferred submission by submitting in the submit
+ // event handler. To prevent the form from being submitted twice, the
+ // <form> tag contains the attribute |onsubmit="return false;"| to cancel
+ // the original submission.
+ form.submit();
+
+ if (aTestActions.setattr) {
+ for (let {attr, value} of aTestActions.setattr) {
+ form.setAttribute(attr, value);
+ }
+ }
+ if (aTestActions.unsetattr) {
+ for (let attr of aTestActions.unsetattr) {
+ form.removeAttribute(attr);
+ }
+ }
+ }, {capture: true, once: true});
+
+ // Trigger the above event listener
+ content.document.getElementById(kSubmitButtonId).click();
+
+ // Test that the form was submitted to the correct target (the frame) with
+ // the correct action (kPostActionPage).
+ let frame = content.document.getElementById(kFrameId);
+ await new Promise(resolve => {
+ frame.addEventListener("load", resolve, {once: true});
+ });
+ return frame.contentWindow.location.href;
+ });
+ is(frame_url, kPostActionPage,
+ "Form should have submitted with correct target and action");
+}
+
+add_task(async function() {
+ info("Changing action should flush pending submissions");
+ await runTest({setattr: [{attr: "action", value: "about:blank"}]});
+});
+
+add_task(async function() {
+ info("Changing target should flush pending submissions");
+ await runTest({setattr: [{attr: "target", value: "_blank"}]});
+});
+
+add_task(async function() {
+ info("Unsetting action should flush pending submissions");
+ await runTest({unsetattr: ["action"]});
+});
+
+// On failure, this test will time out rather than failing an assert. When the
+// target attribute is not set, the form will submit the active page, navigating
+// it away and preventing the wait for iframe load from ever finishing.
+add_task(async function() {
+ info("Unsetting target should flush pending submissions");
+ await runTest({unsetattr: ["target"]});
+});
new file mode 100644
--- /dev/null
+++ b/dom/html/test/post_action_page.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html lang="en">
+ <head>
+ <meta charset="utf-8"/>
+ <title>Submission Flush Test Post Action Page</title>
+ </head>
+ <body>
+ <h1>Post Action Page</h1>
+ </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/html/test/submission_flush.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html lang="en">
+ <head>
+ <meta charset="utf-8"/>
+ <title>Submission Flush Test</title>
+ </head>
+ <body>
+ <form id="test_form" action="post_action_page.html" target="form_target" method="POST" onsubmit="return false;">
+ <button type="submit" id="submit_button">Submit</button>
+ </form>
+ <iframe name="form_target" id="test_frame"></iframe>
+ </body>
+</html>