1520869 - Setting disabled=true in a checkbox onClick handler cancels onChange handler

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Dennis Schubert [:denschub]]

Attachment: Simple testcase

Given

• a checkbox
• a click listener on the checkbox, setting checkbox.disabled = true;
• a change listener on the checkbox, doing something else

On clicking the checkbox in Firefox, the click handler gets executed, setting disabled properly. However, the change handler is never executed. In Safari, Edge, and Chrome, the change handler is executed.

I have attached a simple testcase, which attempts to set the label's value to "changed" in the onChange handler.

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Alphan, would you like to a look at this? :)

Comment 2

[Alphan Chen [:alchen]]

Sure, I can take a look on this.

Comment 3

[Alphan Chen [:alchen]]

I am trying to find if there is SPEC describing about this behavior.

Comment 4

[Alphan Chen [:alchen]]

There is no description of this scenario in W3C spec.
While discussing with Anne, we also found some different behaviors from other browsers after disabling the element.
For example, FF will still fire events when an element is disabled, but other browsers won't. (it may relate to bug 1443148)

I will investigate what the current model of firing change event for inputElement.

Comment 5

[Alphan Chen [:alchen]]

Anne and I did some experiments.
Chrome and firefox won't fire change event if we change the test as below.
It means that the change event is fired due to the checked value change.

  document.getElementById("checkbox").addEventListener("click", (ev) => {
    ev.target.disabled = true;

•   ev.preventDefault();
  });
  
  checkbox.addEventListener("change", () => {
    document.getElementById("label").textContent = "changed";
  });
  

Comment 6

[Alphan Chen [:alchen]]

There are different behaviors between firefox and other browsers.
For Chrome, the checked is change and the change event is fired.

The behavior of gecko here is like below:

1. [checkbox element is mutable]
   Dispatch Event with EventMessage "eMouseClick".
   Do DoSetChecked() with its oppsite value in "GetEventTargetParent()".

2. Set checkbox element to un-mutable. (disabled = true)

3. [checkbox element is not mutable]
   Dispatch Event with EventMessage "eLegacyDOMActivate".
   Dispatch Event with EventMessage "eFormChange".
   Dispatch Event with EventMessage "eFormCheckboxStateChange".
   GetEventTargetParent() will early reurn due to checkbox is disabled.

We can have a quick fix:
In step 1, set a flag about the value changed.
In step 3, don't do early return if the flag is set.

Comment 7

[Alphan Chen [:alchen]]

[Input activation behavior]
https://html.spec.whatwg.org/multipage/input.html#input-activation-behavior

The activation behavior for input elements are these steps:

1. If this element is not mutable, then return.
2. Run this element's input activation behavior, if any, and do nothing otherwise.

The legacy-pre-activation behavior for input elements are these steps:
2-1 If this element is not mutable, then return.
2-2 (if element's type is checkbox)...
2-3 (if element's type is radio button)...

Since there is a mutable check in legacy-pre-activation behavior, we don't need step 1 when the element is checkbox and radio button.

Comment 8

[Alphan Chen [:alchen]]

Hi Olli,
could you provide some comments or suggestion about this bug?

Comment 9

[Anne (:annevk)]

I filed https://github.com/whatwg/html/issues/4328 to track this on the standards side.

Comment 10

[Olli Pettay [:smaug][[e-mail]](vacation until Nov 16)]

ok, so we follow the current spec, to some extent at least.
I guess simple fix would be to add change (and input?) event to
https://searchfox.org/mozilla-central/rev/c07aaf12f13037c1f5a343d31f8291549e57373f/dom/html/nsGenericHTMLElement.cpp#1963 and in those cases return false.

Assuming other browsers do that, I'd be ok to change the behavior, and then we'll need the spec change.

Comment 11

[Anne (:annevk)]

The models are quite a bit different. Per the specification disabled is only checked during "activation behavior". Gecko checks it when dispatching any event, for each event target in the event path, if I understand it correctly (and then allows a specific list of events to unconditionally bypass this check).

Comment 12

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating webcompat priority whiteboard tags to project flags.

Comment 13

[Jens Stutte [:jstutte]]

Hi Anne, can you please check the priority here?

Comment 14

[Anne (:annevk)]

It seems the webcompat issue got resolved on its own, so I would say this is maintenance that needs to happen at some point. Perhaps Kagami is interested in this as I vaguely recall them fixing related issues in Firefox / the HTML Standard.

Comment 15

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1520869 - Allow input/change events on disabled radio/checkboxes r=smaug

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1756520207e9
Allow input/change events on disabled radio/checkboxes r=smaug

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/27276 for changes under testing/web-platform/tests

Comment 18

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/1756520207e9

Comment 19

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot