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.

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

html: Add tests for SubmitEvent interface #19562

Merged
merged 5 commits into from
Nov 1, 2019
Merged

Conversation

tkent-google
Copy link
Contributor

Specification PR: whatwg/html#4984

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look great, ideally we also have a manual test (maybe they can be automated now) where we hit enter inside a text input which causes the browser to select the submitter.

@tkent-google
Copy link
Contributor Author

ideally we also have a manual test (maybe they can be automated now) where we hit enter inside a text input which causes the browser to select the submitter.

A way to trigger implicit submission is not standardized. https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#implicit-submission

I remember we had a discussion about how to handle tests for such de-facto behavior in WPT, but I don't remember the result.

@annevk
Copy link
Member

annevk commented Oct 15, 2019

I guess we can add a manual test for now? Both scenarios of having a submit button and not having one are interesting, as in the latter case the submitter would be the form element which I'm not sure is what we want to expose through this API?

@domenic
Copy link
Member

domenic commented Oct 15, 2019

I remember we had a discussion about how to handle tests for such de-facto behavior in WPT, but I don't remember the result.

My memory is that we put them in separate files with a suffix like .optional and a HTML comment explaining how it is testing a common or "should" behavior, but that failures can be OK because it is not testing a fully-standardized behavior.

@jgraham
Copy link
Contributor

jgraham commented Oct 15, 2019

I remember we had a discussion about how to handle tests for such de-facto behavior in WPT, but I don't remember the result.

My memory is that we put them in separate files with a suffix like .optional and a HTML comment explaining how it is testing a common or "should" behavior, but that failures can be OK because it is not testing a fully-standardized behavior.

https://web-platform-tests.org/writing-tests/file-names.html .optional for SHOULD-type behaviour, .tentative for not yet specced behaviour.

@tkent-google
Copy link
Contributor Author

My memory is that we put them in separate files with a suffix like .optional and a HTML comment explaining how it is testing a common or "should" behavior, but that failures can be OK because it is not testing a fully-standardized behavior.

https://web-platform-tests.org/writing-tests/file-names.html .optional for SHOULD-type behaviour, .tentative for not yet specced behaviour.

Thanks! I added implicit-submission.optional.html.

domenic pushed a commit to whatwg/html that referenced this pull request Oct 31, 2019
It has a submitter attribute, so this fixes #3195.

As part of this, update requestSubmit() to accept null, so that
requestSubmit(e.submitter) always works. (e.submitter is null for
implicit submission, per the discussion in
#3195 (comment).)

Tests: web-platform-tests/wpt#19562
@tkent-google tkent-google merged commit 61c2804 into master Nov 1, 2019
@tkent-google tkent-google deleted the tkent-submitter branch November 1, 2019 06:24
zcorpan pushed a commit to whatwg/html that referenced this pull request Nov 6, 2019
It has a submitter attribute, so this fixes #3195.

As part of this, update requestSubmit() to accept null, so that
requestSubmit(e.submitter) always works. (e.submitter is null for
implicit submission, per the discussion in
#3195 (comment).)

Tests: web-platform-tests/wpt#19562
Copy link

@rrp372 rrp372 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants