Add FormData.constructor submitter tests#37895
Add FormData.constructor submitter tests#37895annevk merged 1 commit intoweb-platform-tests:masterfrom
Conversation
|
Although this file contains a bunch of existing |
| let entries = [...formData.entries()]; | ||
|
|
||
| assert_deep_equals(entries, [["n1", "v1"], ["n3", "v3"]]); | ||
| }, 'The constructed FormData object should not contain entries for an unactivated Image Button submitter'); |
There was a problem hiding this comment.
I believe this test is correct based on my reading of the spec, i.e. before this algorithm was invoked the user indicated a coordinate would imply that no entries should be added in the negative case. But it could also be argued that that is undefined behavior, since it doesn't explicitly continue in the prose.
In looking at implementations of the algorithm however, it seems that the major implementors (Blink, WebKit, Gecko) default the coordinate to 0:0, e.g. see screenshots here and here (an un-activated Image Button submitter is used when constructing the entry list).
Would y'all recommend:
- Leaving this test in (and getting that fixed in implementations), OR
- Removing the test and considering it undefined behavior, since the spec is slightly unclear, OR
- Changing the test to expect a 0:0 coordinate, and clarifying the HTML spec, OR
- Something else?
As far as fixing implementations to check the activated-ness, my hunch is that should be fairly safe/straightforward, since in normal submissions the submitter would have activated. I think it's only with the new FormData submitter parameter that there might be an unactivated submitter.
There was a problem hiding this comment.
I think 3 makes the most sense here. Thanks for catching that!
There was a problem hiding this comment.
https://software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20html%3E%0A%3Cform%20action%3D%2Futilities%2Fcgi%2Ftest-tools%2Fecho%3E%0A%20%3Cinput%20type%3Dimage%3E%0A%3C%2Fform%3E%0A%3Cscript%3E%0Aonload%20%3D%20()%20%3D%3E%20document.querySelector(%22input%22).click()%0A%3C%2Fscript%3E also shows this is a clear bug in HTML. I'm surprised we didn't catch this before.
There was a problem hiding this comment.
So @jenseng as for next steps:
- Leave the test in, but change the pass condition.
- File an issue against or directly PR HTML.
Given my demo above this is a (long overdue given how fundamental the algorithm is) bug fix.
There was a problem hiding this comment.
@annevk sounds good, I'll tweak the test accordingly and file a separate issue/PR.
One slight clarification though, does your test actually demonstrate a bug in the spec? The click() method should trigger the activation behavior, and when an Image Button is activated it sets the coordinate to (0,0) if the user didn't explicitly pick one.
I think the ambiguity/discrepancy is just for Image Button submitters that were not explicitly activated (e.g. when using aForm.requestSubmit(aSubmitter) or the proposed new FormData(aForm, aSubmitter). Let me know If I'm missing something.
There was a problem hiding this comment.
Oh lovely, more action-at-a-distance. You're quite right.
https://software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20html%3E%0A%3Cform%20action%3D%2Futilities%2Fcgi%2Ftest-tools%2Fecho%3E%0A%20%3Cinput%20type%3Dimage%3E%0A%3C%2Fform%3E%0A%3Cscript%3E%0Aonload%20%3D%20()%20%3D%3E%20document.querySelector(%22form%22).requestSubmit(document.querySelector(%22input%22))%0A%3C%2Fscript%3E (using requestSubmit()) demonstrates it better.
I guess once we have the inline defaulting we should simplify that activation behavior language at the same time.
a3b64db to
84a5146
Compare
|
Ok, made the Also refactored/moved the new tests over into All tests pass on my Blink/Gecko/WebKit POCs 🥳 |
This patch adds support for the new submitter argument to the FormData constructor. Spec: https://xhr.spec.whatwg.org/#interface-formdata Spec PR (merged): whatwg/xhr#366 WPT PR: #37895 Bug: 1410542 Change-Id: If17f782f75ae40ae21241c169afc52761fc89544
https://bugs.webkit.org/show_bug.cgi?id=251220 Reviewed by Chris Dumez Implement the new submitter parameter to the FormData constructor Spec PR: whatwg/xhr#366 WPT PR: web-platform-tests/wpt#37895 Test: imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter.html * LayoutTests/imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter.html: Added. * Source/WebCore/html/DOMFormData.cpp: (WebCore::DOMFormData::create): add submitter support * Source/WebCore/html/DOMFormData.h: update signature * Source/WebCore/html/DOMFormData.idl: update interface * Source/WebCore/html/HTMLFormElement.cpp: (WebCore::HTMLFormElement::requestSubmit): improve error messages# Canonical link: https://commits.webkit.org/259558@main
…bidl,smaug Also improve error messages for current submitter validations Spec PR: whatwg/xhr#366 WPT PR: web-platform-tests/wpt#37895 Differential Revision: https://phabricator.services.mozilla.com/D167576
This patch adds support for the new submitter argument to the FormData constructor. Spec: https://xhr.spec.whatwg.org/#interface-formdata Spec PR (merged): whatwg/xhr#366 WPT PR: web-platform-tests/wpt#37895 Bug: 1410542 Change-Id: If17f782f75ae40ae21241c169afc52761fc89544 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189297 Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1100734}
This patch adds support for the new submitter argument to the FormData constructor. Spec: https://xhr.spec.whatwg.org/#interface-formdata Spec PR (merged): whatwg/xhr#366 WPT PR: #37895 Bug: 1410542 Change-Id: If17f782f75ae40ae21241c169afc52761fc89544 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189297 Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1100734}
This patch adds support for the new submitter argument to the FormData constructor. Spec: https://xhr.spec.whatwg.org/#interface-formdata Spec PR (merged): whatwg/xhr#366 WPT PR: #37895 Bug: 1410542 Change-Id: If17f782f75ae40ae21241c169afc52761fc89544 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189297 Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1100734}
…bidl,smaug Also improve error messages for current submitter validations Spec PR: whatwg/xhr#366 WPT PR: web-platform-tests/wpt#37895 Differential Revision: https://phabricator.services.mozilla.com/D167576
…ata constructor, a=testonly Automatic update from web-platform-tests Add optional submitter argument to FormData constructor This patch adds support for the new submitter argument to the FormData constructor. Spec: https://xhr.spec.whatwg.org/#interface-formdata Spec PR (merged): whatwg/xhr#366 WPT PR: web-platform-tests/wpt#37895 Bug: 1410542 Change-Id: If17f782f75ae40ae21241c169afc52761fc89544 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189297 Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1100734} -- wpt-commits: 181fc870aeae110d95537bac969f76caa55d5112 wpt-pr: 38242
…ata constructor, a=testonly Automatic update from web-platform-tests Add optional submitter argument to FormData constructor This patch adds support for the new submitter argument to the FormData constructor. Spec: https://xhr.spec.whatwg.org/#interface-formdata Spec PR (merged): whatwg/xhr#366 WPT PR: web-platform-tests/wpt#37895 Bug: 1410542 Change-Id: If17f782f75ae40ae21241c169afc52761fc89544 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189297 Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1100734} -- wpt-commits: 181fc870aeae110d95537bac969f76caa55d5112 wpt-pr: 38242
This patch adds support for the new submitter argument to the FormData constructor. Spec: https://xhr.spec.whatwg.org/#interface-formdata Spec PR (merged): whatwg/xhr#366 WPT PR: #37895 Bug: 1410542 Change-Id: If17f782f75ae40ae21241c169afc52761fc89544 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189297 Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1100734}
Tests: web-platform-tests/wpt#37895. Closes #262.
Add optional submitter parameter to FormData constructor
https://bugs.webkit.org/show_bug.cgi?id=251220
Reviewed by Chris Dumez
Implement the new submitter parameter to the FormData constructor
Spec PR: whatwg/xhr#366
WPT PR: web-platform-tests/wpt#37895
Test: imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter.html
* LayoutTests/imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/xhr/formdata/constructor-submitter.html: Added.
* Source/WebCore/html/DOMFormData.cpp:
(WebCore::DOMFormData::create): add submitter support
* Source/WebCore/html/DOMFormData.h: update signature
* Source/WebCore/html/DOMFormData.idl: update interface
* Source/WebCore/html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::requestSubmit): improve error messages#
Canonical link: https://commits.webkit.org/259558@main
Canonical link: https://commits.webkit.org/259548.340@safari-7615-branch
Add 7 new tests around the
FormData.constructorsubmitterargument (new feature proposed here). This is ready for review in the context of that proposal, but shouldn't be merged until it is accepted.FormData construction should allow a null submitterandThe constructed FormData object should not contain an entry for an unnamed submit button submitter), as can be seen here.