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: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.7 | ||||||||||
Attachments: |
|
Description
balders
2012-04-24 00:42:32 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. Created attachment 150232 [details]
Patch
@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. 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. Created attachment 151294 [details]
Patch
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. Am I right to assume that these tests pass in Firefox? (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. 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. Created attachment 151450 [details]
Patch
Comment on attachment 151450 [details]
Patch
ok
Comment on attachment 151450 [details] Patch Clearing flags on attachment: 151450 Committed r122224: <http://trac.webkit.org/changeset/122224> All reviewed patches have been landed. Closing bug. |