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

Synthetic click events and event dispatch #3974

Merged
merged 1 commit into from
Oct 20, 2016
Merged

Conversation

annevk
Copy link
Member

@annevk annevk commented Oct 13, 2016

This is testing some of the scenarios discussed in
whatwg/dom#325 and specified in
whatwg/dom#342.

@wpt-pr-bot
Copy link
Collaborator

Reviewers for this pull request are: @Ms2ger, @ayg, @jdm, @plehegar, @zcorpan, and @zqzhang.

@foolip
Copy link
Member

foolip commented Oct 13, 2016

@dtapuska can you review, and perhaps contribute tests from Blink in this area if there are any?

@foolip
Copy link
Member

foolip commented Oct 13, 2016

As a general point, I wonder if these tests should all be sync and more explicitly verify that the behavior indeed happens synchronously. Some of the tests have t.done() at the end of the outermost function, and nothing that happens after that point affects the results, so it's kind of implied (but far from obvious) that the t.step_func's run before that point.

@annevk
Copy link
Member Author

annevk commented Oct 13, 2016

They cannot use test() due to event handlers using "report the exception". Would result in uncaught exceptions all over. (Actually did when I first wrote these.)

@foolip
Copy link
Member

foolip commented Oct 13, 2016

Oh, so exceptions thrown in the event handler wouldn't fail the right test, I see.

@dtapuska
Copy link
Contributor

dtapuska commented Oct 17, 2016

@annevk the blink ones test a disabled control and that stopPropagation doesn't change activation behaviour. Can you add those to your test?

@foolip
Copy link
Member

foolip commented Oct 18, 2016

I've filed https://bugs.chromium.org/p/chromium/issues/detail?id=656976 to get those tests into web-platform-tests. If it's not done by a Blink engineer, we'll probably end up with duplicated tests. It's still too hard to work with web-platfom-tests in Blink, but when it's easier in Blink and elsewhere, I think it'll be reasonable to ask implementers to volunteer to write the web-platform-tests changes more often than not.

child.dispatchEvent(new MouseEvent("click")) // does not bubble
assert_false(input.checked)
t.done()
}, "only look at parents when event bubbles")
Copy link
Member

Choose a reason for hiding this comment

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

This test name is confusing to me; it reads like the event target should be ignored when the event bubbles. How about "ignore parent when event does not bubble"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't that just saying the same thing (the event target in the test doesn't have activation behavior)?

Copy link
Member

Choose a reason for hiding this comment

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

I mean I read it as the "only" refers to the parents, rather than the bubbles. (In this test the event doesn't bubble.) Anyway, not a critical issue :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

I could move "only" to just before "when" I suppose.

Copy link
Member

Choose a reason for hiding this comment

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

WFM

dump.appendChild(input)
input.onclick = t.step_func_done(() => assert_true(input.checked))
input.click()
}, "basic with click()")
Copy link
Member

Choose a reason for hiding this comment

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

Please add a blank line between tests

@annevk
Copy link
Member Author

annevk commented Oct 20, 2016

@dtapuska I assume @foolip took care of your comment by filing that follow up issue.

This is testing some of the scenarios discussed in
whatwg/dom#325 and specified in
whatwg/dom#342.
@Ms2ger Ms2ger force-pushed the annevk/dispatch-click-magic branch from 120a9b9 to c36310c Compare October 20, 2016 11:48
@Ms2ger Ms2ger merged commit 933a7cc into master Oct 20, 2016
@Ms2ger Ms2ger deleted the annevk/dispatch-click-magic branch October 20, 2016 12:29
annevk added a commit to whatwg/html that referenced this pull request Oct 20, 2016
There are two major changes here:

* Any click event can cause activation behavior to run. No exceptions.
* The "click in progress flag" is now restricted to the click() method, as it already is in Firefox.

The other changes are editorial:

* "Activation behavior" is now associated with an object at all times (and cannot be conditionally associated)
* "Legacy-pre-activation behavior" and "legacy-canceled-activation behavior" are now more clearly only relevant for input elements in either the Checkbox or Radio Button state.
* Various concepts such as "run synthetic click activation steps" and "nearest activatable element" are gone as they are obsoleted by the DOM Standard or the aforementioned normative changes.

Tests related to this change:

* web-platform-tests/wpt#3974
* web-platform-tests/wpt#4034

Changes to the DOM Standard that were necessary for this change:

* whatwg/dom#342
* whatwg/dom#346

Fixes #1394.
alice pushed a commit to alice/html that referenced this pull request Jan 8, 2019
There are two major changes here:

* Any click event can cause activation behavior to run. No exceptions.
* The "click in progress flag" is now restricted to the click() method, as it already is in Firefox.

The other changes are editorial:

* "Activation behavior" is now associated with an object at all times (and cannot be conditionally associated)
* "Legacy-pre-activation behavior" and "legacy-canceled-activation behavior" are now more clearly only relevant for input elements in either the Checkbox or Radio Button state.
* Various concepts such as "run synthetic click activation steps" and "nearest activatable element" are gone as they are obsoleted by the DOM Standard or the aforementioned normative changes.

Tests related to this change:

* web-platform-tests/wpt#3974
* web-platform-tests/wpt#4034

Changes to the DOM Standard that were necessary for this change:

* whatwg/dom#342
* whatwg/dom#346

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

Successfully merging this pull request may close these issues.

6 participants