Bug 134545 – input type=range element should only fire change events after committing a value

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

input type=range element should only fire change events after committing a  value

Created attachment 234258 [details]
Patch

This patch is intended to bring the input type="range" element in line with the spec, so that change events are dispatched only after the change is committed, i.e., when the user stops dragging the thumb control (cf. http://www.whatwg.org/specs/web-apps/current-work/multipage/common-input-element-attributes.html#event-input-change). The previous behavior was to dispatch a "change" event on every interaction, which is already covered by the "input" event. Note that Firefox and Chrome implement the correct behavior. This patch brings WebKit in line with these implementations.

Created attachment 234260 [details]
A manual test case displaying the value of an input control when "input" and "change" events are dispatched.

[Build Bot]

Comment on attachment 234258 [details]
Patch

Attachment 234258 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5055339456102400

New failing tests:
media/controls-drag-timebar.html
fast/events/onchange-range-slider.html

[Build Bot]

Created attachment 234262 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 234258 [details]
Patch

Attachment 234258 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5204958936825856

New failing tests:
media/controls-drag-timebar.html
fast/events/onchange-range-slider.html

[Build Bot]

Created attachment 234265 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 234258 [details]
Patch

Attachment 234258 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6258274299019264

New failing tests:
media/controls-drag-timebar.html
fast/events/onchange-range-slider.html

[Build Bot]

Created attachment 234276 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Dean Jackson]

Comment on attachment 234258 [details]
Patch

Looks good to me. r+ if you work out the failing tests.

Created attachment 234341 [details]
Patch

[Build Bot]

Comment on attachment 234341 [details]
Patch

Attachment 234341 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5227410207277056

New failing tests:
platform/mac/accessibility/input-slider.html

[Build Bot]

Created attachment 234344 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 234341 [details]
Patch

Attachment 234341 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5171562747527168

New failing tests:
platform/mac/accessibility/input-slider.html

[Build Bot]

Created attachment 234345 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

Created attachment 234348 [details]
Patch

[Build Bot]

Comment on attachment 234348 [details]
Patch

Attachment 234348 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6261832276770816

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html

[Build Bot]

Created attachment 234354 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Dean Jackson]

Comment on attachment 234348 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=234348&action=review

Congratulations. I think this could be the first sibling to commit a patch to WebKit! Just upload the final patch and I'll commit it.

> Source/WebCore/ChangeLog:10
> +        A "change" event was fired every time the slider thumb element was dragged
> +        by the user. The "change" event is now fired only after the thumb
> +        element has stopped moving.

Could you add another paragraph that explains this behaviour matches Firefox and Chrome? I expect we might get random regression reports from this, and it would be good for our bug screeners to know right away that this is correct behaviour (e.g. you noticed that our own media controls were incorrect).

[Dean Jackson]

Can't see how those crashes are caused by this, but they are in media.

Also, double check that's the email you want to use in the ChangeLog.

I cannot reproduce the failing tests on OS X 10.9.3 and do not currently have access to a system running Mountain Lion...

Created attachment 234369 [details]
Patch

Thanks a lot Dean for the review. I hope the latest version of the patch will be satisfactory.

[WebKit Commit Bot]

Comment on attachment 234369 [details]
Patch

Clearing flags on attachment: 234369

Committed r170808: <http://trac.webkit.org/changeset/170808>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

(In reply to comment #24)
> Comment on attachment 234369 [details]
> Patch
> 
> Clearing flags on attachment: 234369
> 
> Committed r170808: <http://trac.webkit.org/changeset/170808>

This caused bug 137805.