whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.11k stars 2.67k forks source link

input[type=checkbox] is mutable even when disabled on every browser #5000

Closed saschanaz closed 4 years ago

saschanaz commented 5 years ago

Test page: https://disabled-form-element.glitch.me/

The page dispatches a synthesized click event when you click the "Click" button to the checkbox and every browser (Firefox, Chrome, and Epiphany (WebKit; I have no Safari 😅)) mutates the disabled checkbox. This violates the spec:

When an input element is disabled, it is not mutable.

But based on the actual behaviors, should this be allowed?

Edit: issue number 5000 🎉

annevk commented 5 years ago

Is this #4328 perhaps?

saschanaz commented 5 years ago

4328 seems to be about elements disabled after legacy-pre-activation behavior but this is more about the elements that already have been disabled.

saschanaz commented 5 years ago

Relevant bugs:

Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1540995 Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1017039#c3

mfreed7 commented 4 years ago

For reference, it appears that all modern browsers will toggle the disabled checkbox, and issue an 'input' event, when a synthetic 'click' event is sent. My guess would be that the spec should be clarified to match existing browser behavior in this case.

saschanaz commented 4 years ago

I explored to modify the spec but got a blocker. 6.4 Activation behavior of elements says:

Certain elements in HTML have an activation behavior, which means that the user can activate them. This is always caused by a click event.

... but both Firefox and Chrome trigger it without a click event. It seems that fixing this requires modifying DOM event dispatch algorithm, which would become a broader (and dirtier with exceptions) spec change than I thought. What do you think, would it be worth to do that or do you have an alternative way to fix this?

Ah never mind, DOM does not care about disabled, no need to fix it.

josepharhar commented 2 years ago

This section of the spec says this:

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.

However, the WPT added for this shows that dispatchEvent(new MouseEvent('click')) should still work on disabled form controls. I see that gecko and webkit allow dispatchEvent, but blink does not. I'll try to make blink aligned. Should the sentence in the spec also be updated to allow dispatchEvent, or is dispatchEvent already allowed in the spec somehow?

Also, the code in blink which implements this sentence in the spec about click events will completely cancel the click event and prevent it from being dispatched on any nodes if there is any node in the event path which is a disabled form control. However, it seems like this is not what gecko or webkit are doing - they still emit click events on child and/or parent nodes of the disabled form control for various types of clicks. I assume that our new test also permits click events on child and/or parent nodes of disabled form controls: https://github.com/web-platform-tests/wpt/pull/32381 If we update this sentence, this should probably be spelled out too.

saschanaz commented 2 years ago

Should the sentence in the spec also be updated to allow dispatchEvent, or is dispatchEvent already allowed in the spec somehow?

The spec limits the behavior to events from user interaction task source, so I believe it does not block dispatchEvent.

However, it seems like this is not what gecko or webkit are doing - they still emit click events on child and/or parent nodes of the disabled form control for various types of clicks.

Per https://wpt.fyi/results/html/semantics/forms/event-propagate-disabled.html?diff&filter=ADC&run_id=5757896716451840&run_id=5695315419070464 Gecko does not do that, do you have a test case that shows the behavior? It completely blocks pointerdown/up/click per my observation.

I assume that our new test also permits click events on child and/or parent nodes of disabled form controls: https://github.com/web-platform-tests/wpt/pull/32381 If we update this sentence, this should probably be spelled out too.

Yup, I'll prepare a spec PR soon when I get confident enough that we have enough consensus around the tests.

josepharhar commented 2 years ago

The spec limits the behavior to events from user interaction task source, so I believe it does not block dispatchEvent.

Awesome!

Gecko does not do that, do you have a test case that shows the behavior? It completely blocks pointerdown/up/click per my observation.

<div id=testparent>
  <button disabled id=test>disabled button
    <span id=testchild style="border:1px solid black">child span</span>
  </button>
</div>

<div id=output></div>

<script>
  function log(str) {
    output.insertAdjacentHTML('beforeend', `<div>${str}</div>`);
  }
  test.addEventListener('click', () => log('click handler'));
  testparent.addEventListener('click', () => log('parent click handler'));
  testchild.addEventListener('click', () => log('child click handler'));
</script>

Physically clicking "child span" fires the click event on no elements in blink, fires only on the child element in gecko, and fires on the child and parent elements in webkit.

After my fix to implement https://github.com/web-platform-tests/wpt/pull/32381 in blink, then blink webkit and gecko will all have the same behavior for testchild.click() and testchild.dispatchEvent(new MouseEvent('click')).

saschanaz commented 2 years ago

Physically clicking "child span" fires the click event on no elements in blink,

With --enable-experimental-web-platform-features, right? Without the flag it shows the same behavior as WebKit.

fires only on the child element in gecko, and fires on the child and parent elements in webkit.

After my fix to implement https://github.com/web-platform-tests/wpt/pull/32381 in blink, then blink webkit and gecko will all have the same behavior for testchild.click() and testchild.dispatchEvent(new MouseEvent('click')).

👍

josepharhar commented 2 years ago

With --enable-experimental-web-platform-features, right? Without the flag it shows the same behavior as WebKit.

Oof, I don't think the flag should be taking away events like that, I'll have to look into it. Is there a test case in https://github.com/web-platform-tests/wpt/pull/32381 which covers this? What is the desired behavior?

saschanaz commented 2 years ago

We want things to bubble, so in this case we need to follow what WebKit (and Blink by default!) does.

Is there a test case in https://github.com/web-platform-tests/wpt/pull/32381 which covers this?

No, I'll have to add one...

josepharhar commented 2 years ago

I think the test titled Trusted click on <button disabled=""><span class="target">Span</span></button>, observed from <body> should cover this, right...?

saschanaz commented 2 years ago

Hmm yes, I mean the one that "checks whether the child element gets the event" does not exist and probably worth having one.