Skip to content

Event dispatching on disabled form controls - #13837

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
cdumez:disabled_elements_events
May 16, 2023
Merged

Event dispatching on disabled form controls#13837
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
cdumez:disabled_elements_events

Conversation

@cdumez

@cdumez cdumez commented May 13, 2023 •
edited by webkit-early-warning-system
Loading

Copy link
Copy Markdown
Contributor

9ed97a1

Event dispatching on disabled form controls
https://bugs.webkit.org/show_bug.cgi?id=251246
rdar://104727624

Reviewed by Ryosuke Niwa.

Start dispatching events on disabled form controls behind a runtime flag.

This patch also makes "click", "mousedown", and "mouseup" events have their event
paths never include any disabled form controls or ancestors of disabled
form controls. This aligns with what Blink did in:
- https://chromium-review.googlesource.com/c/chromium/src/+/3929060
And is being discussed at:
- whatwg/html#5886

Also make it so that fieldset elements can no longer be disabled, as per:
- https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#enabling-and-disabling-form-controls:-the-disabled-attribute

This gets us closer to the specification and Chrome & Firefox's behavior.

This is part of Interop 2023.

All new behavior is behind a runtime feature flag, currently off by default.

* LayoutTests/fast/css/pseudo-indeterminate-radio-buttons-basics-expected.html:
* LayoutTests/fast/forms/disabled-mousedown-event-expected.txt:
* LayoutTests/fast/forms/disabled-mousedown-event.html:
* LayoutTests/fast/forms/fieldset/fieldset-disabled-expected.txt:
* LayoutTests/fast/forms/fieldset/fieldset-disabled.html:
Resync these tests from Blink.

* LayoutTests/imported/w3c/web-platform-tests/dom/events/Event-dispatch-on-disabled-elements.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/disabled-elements/disabled-event-dispatch.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/disabled-elements/fieldset-event-propagation.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/selectors/pseudo-classes/disabled.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/selectors/pseudo-classes/enabled.html:
Resync these tests from WPT and rebaseline now that more checks are passing.

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/disabled-elements/event-propagate-disabled.tentative-expected.txt:
Rebaseline now that more checks are passing. Note that some subtests are failing but I believe the test needs updating.
These subtests are failing in Blink as well:
https://chromium.googlesource.com/chromium/src.git/+/refs/heads/main/third_party/blink/web_tests/external/wpt/html/semantics/disabled-elements/event-propagate-disabled.tentative.html.ini

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/dom/Element.cpp:
(WebCore::Element::dispatchMouseEvent):
* Source/WebCore/dom/EventContext.cpp:
(WebCore::EventContext::handleLocalEvents const):
* Source/WebCore/dom/EventDispatcher.cpp:
(WebCore::EventDispatcher::dispatchEvent):
* Source/WebCore/dom/EventPath.cpp:
(WebCore::EventPath::adjustForDisabledFormControl):
* Source/WebCore/dom/EventPath.h:
* Source/WebCore/html/HTMLFieldSetElement.cpp:
(WebCore::HTMLFieldSetElement::isDisabledFormControl const):
* Source/WebCore/html/HTMLFieldSetElement.h:
* Source/WebCore/html/HTMLFormControlElement.h:

Canonical link: https://commits.webkit.org/264098@main

5ca3987

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ⏳ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
⏳ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ❌ 🧪 gtk-wk2
✅ 🛠 🧪 jsc ⏳ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 jsc-armv7-tests
✅ 🛠 🧪 merge ✅ 🛠 watch ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@cdumez cdumez self-assigned this May 13, 2023
@cdumez cdumez added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label May 13, 2023
@webkit-early-warning-system

webkit-early-warning-system commented May 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash 59e6a8d)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🧪 gtk-wk2
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv ❌ 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 13, 2023
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label May 15, 2023
@cdumez
cdumez force-pushed the disabled_elements_events branch from 59e6a8d to d657662 Compare May 15, 2023 15:41
@webkit-early-warning-system

webkit-early-warning-system commented May 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash d657662)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
⏳ 🧪 ios-wk2-wpt   🧪 mac-wk1 ⏳ 🧪 gtk-wk2
✅ 🛠 🧪 jsc ⏳ 🧪 api-ios   🧪 mac-wk2   🧪 api-gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim   🧪 mac-wk2-stress ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@cdumez cdumez changed the title [WIP] Event dispatching on disabled form controls Event dispatching on disabled form controls May 15, 2023
@cdumez
cdumez force-pushed the disabled_elements_events branch from d657662 to ca3b0bd Compare May 15, 2023 15:55
@webkit-early-warning-system

webkit-early-warning-system commented May 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash ca3b0bd)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
  🧪 ios-wk2-wpt ❌ 🧪 mac-wk1   🧪 gtk-wk2
✅ 🛠 🧪 jsc ⏳ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@nt1m

nt1m commented May 15, 2023

Copy link
Copy Markdown
Member

Thanks for doing this! The radar link in the commit message needs updating fwiw

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 15, 2023
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label May 15, 2023
@cdumez
cdumez force-pushed the disabled_elements_events branch from ca3b0bd to b0b7667 Compare May 15, 2023 18:04
@webkit-early-warning-system

webkit-early-warning-system commented May 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash b0b7667)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings   🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
❌ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🧪 gtk-wk2
✅ 🛠 🧪 jsc   🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@cdumez
cdumez marked this pull request as ready for review May 15, 2023 20:18
@cdumez
cdumez requested a review from rniwa as a code owner May 15, 2023 20:18
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 15, 2023
rniwa
rniwa reviewed May 15, 2023
View reviewed changes
Comment thread Source/WebCore/dom/EventPath.cpp Outdated

@rniwa rniwa May 15, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where in the spec does it say to do this?

@cdumez cdumez May 15, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As far as I know it doesn't currently but this is Firefox behavior. This is also what Blink did more recently to align:

This is what's being discussed here (seemed to be the most popular behavior):

With us doing this too, all 3 engines should agree on this behavior now.

@cdumez cdumez May 15, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added this information to the commit log.

@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label May 15, 2023
@cdumez
cdumez force-pushed the disabled_elements_events branch from b0b7667 to 5ca3987 Compare May 15, 2023 20:53
@webkit-early-warning-system

webkit-early-warning-system commented May 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on current version of this PR (hash 5ca3987)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ⏳ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
⏳ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ❌ 🧪 gtk-wk2
✅ 🛠 🧪 jsc ⏳ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 jsc-armv7-tests
✅ 🛠 🧪 merge ✅ 🛠 watch ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@cdumez
cdumez requested a review from annevk May 15, 2023 21:19
@cdumez cdumez added the merge-queue Applied to send a pull request to merge-queue label May 16, 2023
@cdumez
Event dispatching on disabled form controls
9ed97a1 
https://bugs.webkit.org/show_bug.cgi?id=251246
rdar://104727624

Reviewed by Ryosuke Niwa.

Start dispatching events on disabled form controls behind a runtime flag.

This patch also makes "click", "mousedown", and "mouseup" events have their event
paths never include any disabled form controls or ancestors of disabled
form controls. This aligns with what Blink did in:
- https://chromium-review.googlesource.com/c/chromium/src/+/3929060
And is being discussed at:
- whatwg/html#5886

Also make it so that fieldset elements can no longer be disabled, as per:
- https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#enabling-and-disabling-form-controls:-the-disabled-attribute

This gets us closer to the specification and Chrome & Firefox's behavior.

This is part of Interop 2023.

All new behavior is behind a runtime feature flag, currently off by default.

* LayoutTests/fast/css/pseudo-indeterminate-radio-buttons-basics-expected.html:
* LayoutTests/fast/forms/disabled-mousedown-event-expected.txt:
* LayoutTests/fast/forms/disabled-mousedown-event.html:
* LayoutTests/fast/forms/fieldset/fieldset-disabled-expected.txt:
* LayoutTests/fast/forms/fieldset/fieldset-disabled.html:
Resync these tests from Blink.

* LayoutTests/imported/w3c/web-platform-tests/dom/events/Event-dispatch-on-disabled-elements.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/disabled-elements/disabled-event-dispatch.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/disabled-elements/fieldset-event-propagation.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/selectors/pseudo-classes/disabled.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/selectors/pseudo-classes/enabled.html:
Resync these tests from WPT and rebaseline now that more checks are passing.

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/disabled-elements/event-propagate-disabled.tentative-expected.txt:
Rebaseline now that more checks are passing. Note that some subtests are failing but I believe the test needs updating.
These subtests are failing in Blink as well:
https://chromium.googlesource.com/chromium/src.git/+/refs/heads/main/third_party/blink/web_tests/external/wpt/html/semantics/disabled-elements/event-propagate-disabled.tentative.html.ini

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/dom/Element.cpp:
(WebCore::Element::dispatchMouseEvent):
* Source/WebCore/dom/EventContext.cpp:
(WebCore::EventContext::handleLocalEvents const):
* Source/WebCore/dom/EventDispatcher.cpp:
(WebCore::EventDispatcher::dispatchEvent):
* Source/WebCore/dom/EventPath.cpp:
(WebCore::EventPath::adjustForDisabledFormControl):
* Source/WebCore/dom/EventPath.h:
* Source/WebCore/html/HTMLFieldSetElement.cpp:
(WebCore::HTMLFieldSetElement::isDisabledFormControl const):
* Source/WebCore/html/HTMLFieldSetElement.h:
* Source/WebCore/html/HTMLFormControlElement.h:

Canonical link: https://commits.webkit.org/264098@main
@webkit-commit-queue
webkit-commit-queue force-pushed the disabled_elements_events branch from 5ca3987 to 9ed97a1 Compare May 16, 2023 03:33
@webkit-commit-queue

webkit-commit-queue commented May 16, 2023

Copy link
Copy Markdown
Collaborator

Committed 264098@main (9ed97a1): https://commits.webkit.org/264098@main

Reviewed commits have been landed. Closing PR #13837 and removing active labels.

@webkit-commit-queue
webkit-commit-queue merged commit 9ed97a1 into WebKit:main May 16, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 16, 2023
@hjanuschka hjanuschka mentioned this pull request Mar 1, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rniwa rniwa rniwa approved these changes
@annevk annevk Awaiting requested review from annevk

Assignees

@cdumez cdumez

Labels

DOM For bugs specific to XML/HTML DOM elements (including parsing).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@cdumez @webkit-early-warning-system @nt1m @webkit-commit-queue @rniwa @webkit-ews-buildbot