1206616 - <input> of type=checkbox,radio, file doesn't fire `input` events

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Chris Rebert]

Per the following portions of the HTML specification:
* https://html.spec.whatwg.org/multipage/forms.html#checkbox-state-(type=checkbox):event-input-input
* https://html.spec.whatwg.org/multipage/forms.html#radio-button-state-(type=radio):event-input-input
* https://html.spec.whatwg.org/multipage/forms.html#file-upload-state-(type=file):event-input-input

when the user changes the checkedness of an <input type="checkbox"> or <input type="radio">, or when a user changes the selected files of an <input type="file">, the browser is supposed to fire an `input` event (https://developer.mozilla.org/en-US/docs/Web/Events/input ) at that <input> element.
Firefox 42 doesn't comply with this, and doesn't fire `input` events in these cases.

Testcase: http://output.jsbin.com/jojoji

Comment 1

[Daniele "Mte90" Scasciafratte]

Confirmed also in Firefox 41

Comment 2

[Jessica Jong [:jessica] (inactive)]

I'd like to give it a try!

Comment 3

[Jessica Jong [:jessica] (inactive)]

Per [1], checkbox, radio and file input types have a defined activation behaviour. In the defined activation behaviour described in each of their section, 'input' event is fired before 'change' event, so I'm going to add firing 'input' events before 'change' events.

[1] https://html.spec.whatwg.org/multipage/forms.html#common-input-element-events

Comment 4

[Jessica Jong [:jessica] (inactive)]

Attachment: patch, v1.

Olli, may I have your feedback on this? Thanks.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8743158 [details] [diff] [review]
patch, v1.

So 
https://html.spec.whatwg.org/multipage/forms.html#checkbox-state-(type=checkbox)
https://html.spec.whatwg.org/multipage/forms.html#radio-button-state-(type=radio)
https://html.spec.whatwg.org/multipage/forms.html#file-upload-state-(type=file)


Don't we dispatch the "change" for checkbox and radiobutton 
http://mxr.mozilla.org/mozilla-central/source/dom/html/HTMLInputElement.cpp#3852
Dispatching "input" in the same place would be good.
And the spec talks about activation behavior, which in Gecko terminology is pretty much PostHandleEvent.

Also, dispatching the event in SetCheckedInternal may cause us to dispatch the event twice if 'click' which is changing the state is cancelled (.preventDefault()), at least if I read the code right.

You could add a test for checkbox and radiobutton that (1) when doing click(), the click event is handled before 
 "change" and "input" and (2) that if click event is cancelled, neither of the events is dispatched.

Comment 6

[Jessica Jong [:jessica] (inactive)]

(In reply to Olli Pettay [:smaug] from comment #5)
> Comment on attachment 8743158 [details] [diff] [review]
> patch, v1.
> 
> So 
> https://html.spec.whatwg.org/multipage/forms.html#checkbox-state-
> (type=checkbox)
> https://html.spec.whatwg.org/multipage/forms.html#radio-button-state-
> (type=radio)
> https://html.spec.whatwg.org/multipage/forms.html#file-upload-state-
> (type=file)
> 
> 
> Don't we dispatch the "change" for checkbox and radiobutton 
> http://mxr.mozilla.org/mozilla-central/source/dom/html/HTMLInputElement.
> cpp#3852
> Dispatching "input" in the same place would be good.
> And the spec talks about activation behavior, which in Gecko terminology is
> pretty much PostHandleEvent.

Thanks, that's a useful note.

> 
> Also, dispatching the event in SetCheckedInternal may cause us to dispatch
> the event twice if 'click' which is changing the state is cancelled
> (.preventDefault()), at least if I read the code right.

Yes, you are right, the events were dispatched twice.

> 
> You could add a test for checkbox and radiobutton that (1) when doing
> click(), the click event is handled before 
>  "change" and "input" and (2) that if click event is cancelled, neither of
> the events is dispatched.

I have added a test case for (2), do you think it covers (1) as well? (I'll upload my updated patch right away).

Comment 7

[Jessica Jong [:jessica] (inactive)]

Attachment: patch, v2.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8743646 [details] [diff] [review]
patch, v2.

Is there really no web platform tests for this stuff. If so, surprising.

Comment 9

[Jessica Jong [:jessica] (inactive)]

(In reply to Olli Pettay [:smaug] from comment #8)
> Comment on attachment 8743646 [details] [diff] [review]
> patch, v2.
> 
> Is there really no web platform tests for this stuff. If so, surprising.

Oh, actually there is. They are under testing/web-platform/tests/html/semantics/forms/the-input-element/ and are expected to fail right now.
But after running with the proposed patch, I still get an error:
"assert_false: click()-initiated input event shouldn't be trusted expected false got true"

But per spec, a simple event is trusted [1], unless I am missing something here.

[1] https://html.spec.whatwg.org/multipage/webappapis.html#fire-a-simple-event

Comment 10

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 1: Fire input event for type=checkbox,radio,file. r=smaug

Revised commit message.

Comment 11

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 2: update web platform tests. r=smaug

Updated wpt based on spec: events should be trusted.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

FYI, I filed https://github.com/whatwg/html/issues/1126

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8745170 [details] [diff] [review]
Part 2: update web platform tests. r=smaug

Thanks.
If the specification bug is fixed, checkbox.html and radio.html may be changed back to the old behavior, but that is something for a different bug.

Comment 14

[Jessica Jong [:jessica] (inactive)]

(In reply to Olli Pettay [:smaug] from comment #13)
> Comment on attachment 8745170 [details] [diff] [review]
> Part 2: update web platform tests. r=smaug
> 
> Thanks.
> If the specification bug is fixed, checkbox.html and radio.html may be
> changed back to the old behavior, but that is something for a different bug.

Got it. Thanks for filing the bug.

Comment 15

[Jessica Jong [:jessica] (inactive)]

try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e879e94c2903

Comment 16

[Jessica Jong [:jessica] (inactive)]

try looks good.

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ddb96fbc66c9
https://hg.mozilla.org/integration/mozilla-inbound/rev/477b790ce198

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ddb96fbc66c9
https://hg.mozilla.org/mozilla-central/rev/477b790ce198

Comment 19

[m.kurz]

Is this really fixed?

Please see this test page, there is one failing test:
http://w3c-test.org/html/semantics/forms/the-input-element/checkbox-click-events.html

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

That failing test doesn't seem to have anything to do with input events.
But feel free to file a new bug about the failure.

Comment 21

[m.kurz]

Alright, I opened bug 1434268