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.

Sign up for GitHub

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

Jump to bottom

dispatchEvent now takes into account if an element has been disabled #2681

Closed
wants to merge 1 commit into from
Closed

dispatchEvent now takes into account if an element has been disabled #2681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/jsdom/living/nodes/HTMLElement-impl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ class HTMLElementImpl extends ElementImpl {
}
}

dispatchEvent(eventImpl) {
if (isDisabled(this)) {
return;
}

return super.dispatchEvent(eventImpl);
Copy link
Member

@TimothyGu TimothyGu Oct 14, 2019 •
edited

Choose a reason for hiding this comment

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

This is not correct. https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#enabling-and-disabling-form-controls:-the-disabled-attribute:concept-fe-disabled says:

A form control that is disabled must prevent any click events that are queued on the user interaction task source from being dispatched on the element.

So any special processing must only apply to click events. Edit: the event has to both have the name "click" as well as being a MouseEvent object, as click event is defined to be a MouseEvent.

Additionally, some places call _dispatch directly:

jsdom/lib/jsdom/living/helpers/events.js

Line 17 in e3744f5

return tryImplForWrapper(target)._dispatch(event, legacyTargetOverrideFlag);
It might be better to override _dispatch instead of the more public dispatchEvent.

Copy link
Member

Choose a reason for hiding this comment

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

That can't be the right part of the spec, because the user interaction task source is never involved in jsdom, I don't think.

The click() disabled handling is in the spec at https://html.spec.whatwg.org/multipage/interaction.html#activation:concept-fe-disabled.

For dispatchEvent, I cannot find any support in the spec for this. Furthmore, running the test in #2665 in Firefox shows that it matches jsdom.

So I don't think we should change this in jsdom, but there is a spec interop issue, apparently.

Copy link
Member

Choose a reason for hiding this comment

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

This is whatwg/html#2368

}

click() {
// https://html.spec.whatwg.org/multipage/interaction.html#dom-click
// https://html.spec.whatwg.org/multipage/webappapis.html#fire-a-synthetic-mouse-event
Expand Down
10 changes: 10 additions & 0 deletions test/web-platform-tests/to-upstream/html/editing/activation/click-bail-on-disabled.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@

}, "clicking a disabled button should not cause a click event");

async_test(t => {

const element = document.querySelector("#el1");
element.addEventListener("click", t.unreached_func("click event should never happen"));

element.dispatchEvent(new MouseEvent('click'));
t.step_timeout(() => t.done(), 500);

}, "dispatchEvent should not cause a click event");

async_test(t => {

const element = document.querySelector("#el2");
Expand Down