Bug 84674

Summary:

Input type=range issue with events not being raised when value set in js

Product:

WebKit

Reporter:

balders

Component:

Forms

Assignee:

Kevin Ellis <kevers>

Status:

RESOLVED FIXED

Severity:

Major

CC:

ap, jonlee, kevers, mifenton, rjkroege, tkent, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.7

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

balders

Reported 2012-04-24 00:42:32 PDT

Clicking to the left of the thumb moves the thumb but does not not generate "onclick" or "onchange" event. Second clicks work. <html> <body onload='document.getElementById("myRange").value=1; alert("Primed: Click to the left fails. Click to the right works.");'> <input type="range" id="myRange" min="0" max="3" value="0" onclick="alert('onclick');" onchange="alert('onchange');" /> </body> </html>

Attachments
Patch (8.74 KB, patch) 2012-06-29 11:55 PDT, Kevin Ellis	no flags	Details Formatted Diff Diff
Patch (8.46 KB, patch) 2012-07-09 12:13 PDT, Kevin Ellis	no flags	Details Formatted Diff Diff
Patch (8.43 KB, patch) 2012-07-10 06:29 PDT, Kevin Ellis	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Alexey Proskuryakov

Comment 1 2012-04-24 10:33:03 PDT

Event dispatch certainly appears quite erratic here. And if I change maximum/initial from 3/1 to 30/10, I'm only getting change events, and never click ones.

Kevin Ellis

Comment 2 2012-06-29 11:55:05 PDT

Created attachment 150232 [details] Patch

Kevin Ellis

Comment 3 2012-07-04 06:06:00 PDT

@tkent: Can you please review this patch? There are two issues. First, 'change' events were not reliably fired because of logic to prevent firing duplicate change events for textual input elements. Second, 'click' events were not reliably fired if the slider is repositioned beneath the cursor in the process of clicking. Both issues are addressed in the patch.

Kent Tamura

Comment 4 2012-07-08 22:00:24 PDT

Comment on attachment 150232 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150232&action=review The C++ change looks ok. Need some improvement for tests. > LayoutTests/fast/events/click-range-slider.html:19 > +function log(s) > +{ > + document.getElementById('console').appendChild(document.createTextNode(s + "\n")); > +} Remove this. This is not used. > LayoutTests/fast/events/click-range-slider.html:28 > +if (window.testRunner) { > + testRunner.dumpAsText(); > +} Remove this. js-test-pre.js does it. > LayoutTests/fast/events/click-range-slider.html:55 > + shouldBeEqualToString("String(clickCount)", "3"); Should be: shouldBe("clickCount", "3"); > LayoutTests/fast/events/onchange-range-slider.html:19 > +function log(s) > +{ > + document.getElementById('console').appendChild(document.createTextNode(s + "\n")); > +} Remove this. js-test-pre.js provides debug() function. > LayoutTests/fast/events/onchange-range-slider.html:23 > + log ('PASS: change event fired.'); Use testPassed() provided by js-test-pre.js. > LayoutTests/fast/events/onchange-range-slider.html:30 > +if (window.testRunner) { > + testRunner.dumpAsText(); > +} Remove this. > LayoutTests/fast/events/onchange-range-slider.html:52 > + log('Fail: change event not fired.'); Use testFailed() provided by js-test-pre.js.

Kevin Ellis

Comment 5 2012-07-09 12:13:57 PDT

Created attachment 151294 [details] Patch

Kevin Ellis

Comment 6 2012-07-09 12:26:40 PDT

Comment on attachment 150232 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150232&action=review >> LayoutTests/fast/events/click-range-slider.html:19 >> +} > > Remove this. This is not used. Done. >> LayoutTests/fast/events/click-range-slider.html:28 >> +} > > Remove this. js-test-pre.js does it. Done. >> LayoutTests/fast/events/click-range-slider.html:55 >> + shouldBeEqualToString("String(clickCount)", "3"); > > Should be: > shouldBe("clickCount", "3"); Done. >> LayoutTests/fast/events/onchange-range-slider.html:19 >> +} > > Remove this. js-test-pre.js provides debug() function. Done. >> LayoutTests/fast/events/onchange-range-slider.html:30 >> +} > > Remove this. Done. >> LayoutTests/fast/events/onchange-range-slider.html:52 >> + log('Fail: change event not fired.'); > > Use testFailed() provided by js-test-pre.js. Done.

Alexey Proskuryakov

Comment 7 2012-07-09 12:34:41 PDT

Am I right to assume that these tests pass in Firefox?

Kevin Ellis

Comment 8 2012-07-09 12:58:13 PDT

(In reply to comment #7) > Am I right to assume that these tests pass in Firefox? Does not appear that Firefox supports rendering <input type="range"> as a slider. Bug 344618 - Implement <input type="range"> Tested on Firefox 13.0.1.

Kent Tamura

Comment 9 2012-07-09 20:27:23 PDT

Comment on attachment 151294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151294&action=review > LayoutTests/fast/events/onchange-range-slider.html:24 > +if (window.testRunner) { > + testRunner.dumpAsText(); Should remove this. js-test-pre.js does it.

Kevin Ellis

Comment 10 2012-07-10 06:29:21 PDT

Created attachment 151450 [details] Patch

Kent Tamura

Comment 11 2012-07-10 06:30:26 PDT

Comment on attachment 151450 [details] Patch ok

WebKit Review Bot

Comment 12 2012-07-10 08:51:46 PDT

Comment on attachment 151450 [details] Patch Clearing flags on attachment: 151450 Committed r122224: <http://trac.webkit.org/changeset/122224>

WebKit Review Bot

Comment 13 2012-07-10 08:51:51 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.