Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Form submission: Should check connected twice #2708

Closed
tkent-google opened this issue May 25, 2017 · 6 comments
Closed

Form submission: Should check connected twice #2708

tkent-google opened this issue May 25, 2017 · 6 comments
Labels
topic: forms topic: navigation

Comments

@tkent-google
Copy link
Collaborator

Specification: https://html.spec.whatwg.org/multipage/forms.html#form-submission-algorithm

We should add step 5.3 to check 'connected' again because submit event handler may disconnect the form, and Chrome, Firefox, and Edge have 'connected' check here.
(probably, sanboxing flag too?)

With the following HTML, Chrome, Firefox, and Edge don't submit the form by clicking the submit button.

<body>
<form action="http://www.haun.org/kent/printenv.pl" onsubmit="this.remove();">
<input name="q" value="a">
<input type=submit>
</form>
</body>
@domenic
Copy link
Member

domenic commented May 25, 2017

Agreed, will fix. Are there existing web platform tests perchance?

@tkent-google
Copy link
Collaborator Author

Are there existing web platform tests perchance?

I think no.
We should put new one to html/semantics/forms/form-submission-0/.

@annevk
Copy link
Member

annevk commented May 25, 2017

See #2615 (comment) and that whole issue really. If we don't do connected checks for <a> and <area> and <link> I don't really see the point in doing them for <form>. These are all the elements that can cause navigation, ideally they just work the same way.

@bzbarsky
Copy link
Contributor

Note that step 2 of this algorithm doesn't check for the form being connected, though it seems to think (as of #1383) that it does...

@domenic
Copy link
Member

domenic commented May 25, 2017

Oh goodness, that's embarrassing :).

So it seems like browsers should probably hold off on making changes here until we get agreement on #2615 (i.e. the analogous issue for a/area). Otherwise we might end up removing the connected check entirely. I will update all the bugs I filed asking browsers to hold off.

@domenic
Copy link
Member

domenic commented May 25, 2017

Bugs updated, that I should ping again once we get our story straight:

domenic added a commit that referenced this issue May 31, 2017
@domenic
Fix form connectedness check, but also warn about it
37fa9c1 
#2708 (comment)
pointed out that this check was just broken, so this fixes it. But the
check is also under discussion in #2615 and #2708, pending compat data,
so this adds a warning in the meantime.
@domenic domenic mentioned this issue May 31, 2017
Merged
domenic added a commit that referenced this issue Jun 2, 2017
@domenic
Fix form connectedness check, but also warn about it
df520f8 
#2708 (comment)
pointed out that this check was just broken, so this fixes it. But the
check is also under discussion in #2615 and #2708, pending compat data,
so this adds a warning in the meantime.
@domenic domenic mentioned this issue Aug 23, 2017
Closed
annevk added a commit that referenced this issue Aug 23, 2017
@annevk
check for connectedness twice
5c31784 
fixes #2708
@annevk annevk mentioned this issue Aug 23, 2017
Merged
@annevk annevk closed this as completed in f3c354a Sep 22, 2017
@patricia-gallardo patricia-gallardo mentioned this issue Oct 9, 2018
Closed
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
@domenic
Fix form connectedness check, but also warn about it
7c580fb 
whatwg#2708 (comment)
pointed out that this check was just broken, so this fixes it. But the
check is also under discussion in whatwg#2615 and whatwg#2708, pending compat data,
so this adds a warning in the meantime.
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
@annevk
Centralize checks for <a>, <area>, <form>, and <link>
d9e535c 
Require that they are all connected to a document (except for <a>) that is fully active (including <a>). There is enough difference between implementations that this appears to be web compatible.

Also perform a second connected check for <form>.

Tests:

* web-platform-tests/wpt#5758
* web-platform-tests/wpt#5759
* web-platform-tests/wpt#5761

Fixes whatwg#2615 and fixes whatwg#2708.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic: forms topic: navigation
Milestone
No milestone
Development

No branches or pull requests
4 participants
@domenic @bzbarsky @annevk @tkent-google