RESOLVED FIXED Bug 84674
Input type=range issue with events not being raised when value set in js 
https://bugs.webkit.org/show_bug.cgi?id=84674
Summary Input type=range issue with events not being raised when value set in js
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
DetailsFormatted DiffDiff
Patch (8.46 KB, patch)
2012-07-09 12:13 PDT, Kevin Ellis 		no flags
DetailsFormatted DiffDiff
Patch (8.43 KB, patch)
2012-07-10 06:29 PDT, Kevin Ellis 		no flags
DetailsFormatted DiffDiff
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.