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

Add <form>'s "firing submission events" flag #4621

Merged
merged 4 commits into from
May 15, 2019

Conversation

tkent-google
Copy link
Contributor

@tkent-google tkent-google commented May 14, 2019

This fixes #4620

(See WHATWG Working Mode: Changes for more details.)


/form-control-infrastructure.html ( diff )

@tkent-google tkent-google mentioned this pull request May 14, 2019
3 tasks
tkent-google added a commit to web-platform-tests/wpt that referenced this pull request May 14, 2019
tkent-google added a commit to web-platform-tests/wpt that referenced this pull request May 14, 2019
@annevk
Copy link
Member

annevk commented May 14, 2019

So reentrancy for the invalid event is not prevented?

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM editorially although I am also curious about @annevk's question. We may want to add a test for the invalid event causing reentrancy (e.g. bailing out after 5 recursions).

and probably informed the user of this) then
<span data-x="concept-event-fire">fire an event</span> named
<code data-x="event-invalid">invalid</code> at the <var>form</var> element and then return.
</p></li>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: this wrapping introduces an observable space after the period. I will fix.

Also removes comments that are no longer helpful because the structure is now obvious from the actual spec text.
@tkent-google
Copy link
Contributor Author

So reentrancy for the invalid event is not prevented?

Good point!
Chrome and Firefox seems to set the flag to true after interactive validation. Safari seems to set the flag before interactive validation. IMO Safari's behavior is more reasonable.

I also found none of Chrome, Firefox and Safari dispatches invalid event targeting the <form>.

@domenic
Copy link
Member

domenic commented May 15, 2019

Wow, OK. Can you take care of updating this PR and tests? I think it'll make more sense to just land everything together. This would include a -historical test that there is no invalid event fired at the form

I guess we should name the boolean "firing submission events" or something since it's also about the invalid events.

- Rename the flag; firing submit event -> firing submission events
@tkent-google
Copy link
Contributor Author

Updated this PR. Will update the WPT PR.
I didn't the remove invalid event stuff. IMO it should be discussed separately.

@domenic
Copy link
Member

domenic commented May 15, 2019

I guess that's fair.

@tkent-google tkent-google changed the title Add <form>'s "firing submit event" flag Add <form>'s "firing submission events" flag May 15, 2019
@annevk
Copy link
Member

annevk commented May 15, 2019

I filed #4626 on invalid and <form>.

<var>form</var> and examine the result: if the result is negative (the constraint validation
concluded that there were invalid fields and probably informed the user of this) then <span
data-x="concept-event-fire">fire an event</span> named <code
data-x="event-invalid">invalid</code> at the <var>form</var> element and then return.</p></li>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to reset the flag here too. I'll update the PR tomorrow.

Copy link
Member

Choose a reason for hiding this comment

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

I'll update it now so we can merge; I want to rebase #4597 on top.

@domenic domenic merged commit cb55dca into whatwg:master May 15, 2019
tkent-google added a commit to web-platform-tests/wpt that referenced this pull request May 15, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jun 19, 2019
…entrancy protection, a=testonly

Automatic update from web-platform-tests
html: Add a testcase for submit event reentrancy protection (#16811)

Specification change: whatwg/html#4621

--

wp5At-commits: 7ffd31b4eb23f0d99d5333ae2056b8c6896f463b
wpt-pr: 16811
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Jun 19, 2019
…entrancy protection, a=testonly

Automatic update from web-platform-tests
html: Add a testcase for submit event reentrancy protection (#16811)

Specification change: whatwg/html#4621

--

wp5At-commits: 7ffd31b4eb23f0d99d5333ae2056b8c6896f463b
wpt-pr: 16811
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 23, 2019
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 4, 2019
…entrancy protection, a=testonly

Automatic update from web-platform-tests
html: Add a testcase for submit event reentrancy protection (#16811)

Specification change: whatwg/html#4621

--

wp5At-commits: 7ffd31b4eb23f0d99d5333ae2056b8c6896f463b
wpt-pr: 16811

UltraBlame original commit: b05ea3f35ab4136dd53da0fb0ec41d0819b8a0f7
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 4, 2019
…entrancy protection, a=testonly

Automatic update from web-platform-tests
html: Add a testcase for submit event reentrancy protection (#16811)

Specification change: whatwg/html#4621

--

wp5At-commits: 7ffd31b4eb23f0d99d5333ae2056b8c6896f463b
wpt-pr: 16811

UltraBlame original commit: b05ea3f35ab4136dd53da0fb0ec41d0819b8a0f7
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…entrancy protection, a=testonly

Automatic update from web-platform-tests
html: Add a testcase for submit event reentrancy protection (#16811)

Specification change: whatwg/html#4621

--

wp5At-commits: 7ffd31b4eb23f0d99d5333ae2056b8c6896f463b
wpt-pr: 16811

UltraBlame original commit: b05ea3f35ab4136dd53da0fb0ec41d0819b8a0f7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Reentrancy protection for 'submit' event
3 participants