Bug 134545 - input type=range element should only fire change events after committing a value
Summary: input type=range element should only fire change events after committing a v...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-02 05:41 PDT by pom
Modified: 2014-10-16 21:43 PDT (History)
7 users (show)

See Also:


Attachments
Patch (11.47 KB, patch)
2014-07-02 05:58 PDT, pom
no flags Details | Formatted Diff | Diff
A manual test case displaying the value of an input control when "input" and "change" events are dispatched. (876 bytes, text/html)
2014-07-02 06:10 PDT, pom
no flags Details
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (517.16 KB, application/zip)
2014-07-02 07:25 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (517.86 KB, application/zip)
2014-07-02 08:22 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (480.41 KB, application/zip)
2014-07-02 13:55 PDT, Build Bot
no flags Details
Patch (13.56 KB, patch)
2014-07-03 06:41 PDT, pom
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (503.37 KB, application/zip)
2014-07-03 08:03 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (557.14 KB, application/zip)
2014-07-03 08:16 PDT, Build Bot
no flags Details
Patch (14.53 KB, patch)
2014-07-03 09:39 PDT, pom
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (536.92 KB, application/zip)
2014-07-03 10:45 PDT, Build Bot
no flags Details
Patch (14.88 KB, patch)
2014-07-03 13:28 PDT, pom
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description pom 2014-07-02 05:41:38 PDT
input type=range element should only fire change events after committing a  value
Comment 1 pom 2014-07-02 05:58:33 PDT
Created attachment 234258 [details]
Patch
Comment 2 pom 2014-07-02 06:05:47 PDT
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.
Comment 3 pom 2014-07-02 06:10:30 PDT
Created attachment 234260 [details]
A manual test case displaying the value of an input control when "input" and "change" events are dispatched.
Comment 4 Build Bot 2014-07-02 07:25:37 PDT
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
Comment 5 Build Bot 2014-07-02 07:25:39 PDT
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
Comment 6 Build Bot 2014-07-02 08:22:22 PDT
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
Comment 7 Build Bot 2014-07-02 08:22:24 PDT
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
Comment 8 Build Bot 2014-07-02 13:55:00 PDT
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
Comment 9 Build Bot 2014-07-02 13:55:02 PDT
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
Comment 10 Dean Jackson 2014-07-02 14:09:47 PDT
Comment on attachment 234258 [details]
Patch

Looks good to me. r+ if you work out the failing tests.
Comment 11 pom 2014-07-03 06:41:28 PDT
Created attachment 234341 [details]
Patch
Comment 12 Build Bot 2014-07-03 08:03:48 PDT
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
Comment 13 Build Bot 2014-07-03 08:03:50 PDT
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
Comment 14 Build Bot 2014-07-03 08:16:08 PDT
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
Comment 15 Build Bot 2014-07-03 08:16:10 PDT
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
Comment 16 pom 2014-07-03 09:39:13 PDT
Created attachment 234348 [details]
Patch
Comment 17 Build Bot 2014-07-03 10:45:38 PDT
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
Comment 18 Build Bot 2014-07-03 10:45:40 PDT
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
Comment 19 Dean Jackson 2014-07-03 11:52:33 PDT
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).
Comment 20 Dean Jackson 2014-07-03 11:54:10 PDT
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.
Comment 21 pom 2014-07-03 13:23:40 PDT
I cannot reproduce the failing tests on OS X 10.9.3 and do not currently have access to a system running Mountain Lion...
Comment 22 pom 2014-07-03 13:28:00 PDT
Created attachment 234369 [details]
Patch
Comment 23 pom 2014-07-03 13:29:49 PDT
Thanks a lot Dean for the review. I hope the latest version of the patch will be satisfactory.
Comment 24 WebKit Commit Bot 2014-07-04 12:12:50 PDT
Comment on attachment 234369 [details]
Patch

Clearing flags on attachment: 234369

Committed r170808: <http://trac.webkit.org/changeset/170808>
Comment 25 WebKit Commit Bot 2014-07-04 12:12:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 26 mitz 2014-10-16 21:43:55 PDT
(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.