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 test for event propagation on disabled form controls #32381

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

jgraham
Copy link
Contributor

@jgraham jgraham commented Jan 14, 2022

No description provided.

@jgraham
Copy link
Contributor Author

jgraham commented Jan 14, 2022

https://wpt.fyi/results/html/semantics/forms/event-propagate-disabled.html?label=pr_head&max-count=1&pr=32381

Currently this semi-matches Safari. Chrome seems to not be generating click events at all in this case, but does generate and bubble mousedown and mouseup, and Firefox isn't bubbling any events.

@jgraham jgraham marked this pull request as ready for review March 29, 2022 12:54
@foolip
Copy link
Member

foolip commented Mar 29, 2022

All of the tests fail in all browsers, is that expected? Isn't the behavior of some browser sensible enough to align on?

@jgraham
Copy link
Contributor Author

jgraham commented Mar 29, 2022

@saschanaz might be better to answer that question

@jgraham
Copy link
Contributor Author

jgraham commented Mar 29, 2022

But Chrome seems to special case disabled form controls to not get a click event, whereas per the test results, Firefox and Safari don't give any mouse events on those controls. I think there are some further differences between Firefox and Safari, but I've forgotten exactly what they are.

(I'm going to close and reopen to update the results in case browsers changed since the test was written).

@foolip
Copy link
Member

foolip commented Mar 30, 2022

https://wpt.fyi/results/html/semantics/forms/event-propagate-disabled.html?label=pr_head&max-count=1&pr=32381 is now CRASH in Safari, but still all failing in Chrome and Firefox, so probably the same situation still.

@saschanaz
Copy link
Member

saschanaz commented Mar 30, 2022

Chrome seems to not be generating click events at all in this case,

Chrome does bubble clicks, please check whether you enabled Experimental Web Platform features flag as the flag blocks bubbling. Demo:

data:text/html;charset=utf-8,<body onclick="console.log('click')" onmousedown="console.log('mousedown')"><input type="text" disabled>

(See also whatwg/html#2368 (comment))

@jgraham
Copy link
Contributor Author

jgraham commented Mar 30, 2022

Yeah, we're running this in Chrome dev with the flag set; it sounds like Chrome release might pass the tests as written? Do you happen to know why the experimental flag is blocking (just) click events in this case?

@saschanaz
Copy link
Member

I have no idea, they once wanted to align with Firefox perhaps?

@foolip
Copy link
Member

foolip commented Mar 30, 2022

That "SendMouseEventsDisabledFormControls" feature was added in https://source.chromium.org/chromium/chromium/src/+/af6f27363128a72a89cb581f3073c0e068854308 but was never moved out of experimental.

I've run the test locally with ./wpt run --channel stable chrome html/semantics/forms/event-propagate-disabled.html and all the same tests fail, but instead of getting 2 events (3 expected) there are 0 events fired.

Per https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#enabling-and-disabling-form-controls:-the-disabled-attribute only the "click" event should be dropped, so maybe the per-spec behavior is what Chrome does with "SendMouseEventsDisabledFormControls" enabled is correct, and the test should match that?

@jgraham
Copy link
Contributor Author

jgraham commented Mar 31, 2022

Well I've updated it a bit, but I'm still not sure what behaviour we actually want to align on. In particular Chrome is still failing the tests for select elements.

@josepharhar
Copy link
Contributor

Thanks for making this test! It looks quite comprehensive with bubbling and child elements. Do you think it would be possible to also test mousemove, pointerdown, pointerup, and pointermove?

@josepharhar
Copy link
Contributor

Would it also be possible to look at event.composedPath() in this test? It is likely another compat issue we will need to address

@saschanaz
Copy link
Member

Added it. For now it just includes all nodes as Blink/WebKit do, only to not complicate things before we know what should be done there. I'll adjust it based on the consensus from the next triage meeting.

@saschanaz
Copy link
Member

I added some more tests for internal request.

Short report:

  • Both Firefox and Chrome blocks mousedown/up when disabled during immediate propagation, but does not block click
  • Both Firefox and Chrome changes focuses to the form elements and but Chrome then removes the focus when it's disabled by event listener.

Haven't tested on Safari yet.

@jgraham
Copy link
Contributor Author

jgraham commented Jul 6, 2022

Note that https://wpt.fyi/results/html/semantics/forms?label=pr_head&max-count=1&pr=32381 has the latest results for the PR, including in Safari.

@saschanaz
Copy link
Member

saschanaz commented Jul 6, 2022

(The current assertions, especially in the new tests (-immediate and -by-parent) are not exactly the requirements. I'll adjust them as we better understand what should be done, hopefully after the triage. For now they are to show the differences.)

@saschanaz
Copy link
Member

(I wanted to cover over/out events and then found a separate Gecko bug. I think the behavior is same there anyway, so let's just not complicate this further until it's fixed.)

Filed w3c/pointerevents#454.

@saschanaz saschanaz marked this pull request as draft November 10, 2022 00:41
moz-wptsync-bot pushed a commit that referenced this pull request Nov 11, 2022
Corresponds to the latest consensus and also matches what Chrome shipped behind `--enable-blink-features=SendMouseEventsDisabledFormControls`.

Imported the portion of tests that is directly impacted here from #32381. Others are not directly impacted and thus I'd like to land them separately since there are still some mismatching behavior around `button` element.

Differential Revision: https://phabricator.services.mozilla.com/D161537

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1799565
gecko-commit: dc3970db7ae2ad12db0cfb4ff794fa1f974a74e3
gecko-reviewers: smaug
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 12, 2022
…ly r=smaug

Corresponds to the latest consensus and also matches what Chrome shipped behind `--enable-blink-features=SendMouseEventsDisabledFormControls`.

Imported the portion of tests that is directly impacted here from web-platform-tests/wpt#32381. Others are not directly impacted and thus I'd like to land them separately since there are still some mismatching behavior around `button` element.

Differential Revision: https://phabricator.services.mozilla.com/D161537
moz-wptsync-bot pushed a commit that referenced this pull request Nov 14, 2022
Corresponds to the latest consensus and also matches what Chrome shipped behind `--enable-blink-features=SendMouseEventsDisabledFormControls`.

Imported the portion of tests that is directly impacted here from #32381. Others are not directly impacted and thus I'd like to land them separately since there are still some mismatching behavior around `button` element.

Differential Revision: https://phabricator.services.mozilla.com/D161537

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1799565
gecko-commit: 0f30447455dcbf2c99259f763a443f79fa521757
gecko-reviewers: smaug
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Nov 14, 2022
…ly r=smaug

Corresponds to the latest consensus and also matches what Chrome shipped behind `--enable-blink-features=SendMouseEventsDisabledFormControls`.

Imported the portion of tests that is directly impacted here from web-platform-tests/wpt#32381. Others are not directly impacted and thus I'd like to land them separately since there are still some mismatching behavior around `button` element.

Differential Revision: https://phabricator.services.mozilla.com/D161537
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 14, 2022
…ly r=smaug

Corresponds to the latest consensus and also matches what Chrome shipped behind `--enable-blink-features=SendMouseEventsDisabledFormControls`.

Imported the portion of tests that is directly impacted here from web-platform-tests/wpt#32381. Others are not directly impacted and thus I'd like to land them separately since there are still some mismatching behavior around `button` element.

Differential Revision: https://phabricator.services.mozilla.com/D161537
moz-wptsync-bot pushed a commit that referenced this pull request Nov 14, 2022
Corresponds to the latest consensus and also matches what Chrome shipped behind `--enable-blink-features=SendMouseEventsDisabledFormControls`.

Imported the portion of tests that is directly impacted here from #32381. Others are not directly impacted and thus I'd like to land them separately since there are still some mismatching behavior around `button` element.

Differential Revision: https://phabricator.services.mozilla.com/D161537

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1799565
gecko-commit: 0f30447455dcbf2c99259f763a443f79fa521757
gecko-reviewers: smaug
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Nov 17, 2022
…ly r=smaug

Corresponds to the latest consensus and also matches what Chrome shipped behind `--enable-blink-features=SendMouseEventsDisabledFormControls`.

Imported the portion of tests that is directly impacted here from web-platform-tests/wpt#32381. Others are not directly impacted and thus I'd like to land them separately since there are still some mismatching behavior around `button` element.

Differential Revision: https://phabricator.services.mozilla.com/D161537
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.

None yet

7 participants