RESOLVED FIXED 56059
REGRESSION(r76147): A slider thumb that is styled cannot be programmatically moved 
https://bugs.webkit.org/show_bug.cgi?id=56059
Summary REGRESSION(r76147): A slider thumb that is styled cannot be programmatically ...
piet
Reported 2011-03-09 16:12:27 PST
This is a regression that I noticed after Chrome upgraded from 9 to 10. I'm developing an application that displays videos using the <video> element and custom controls. After upgrading to Chrome 10, the slider that shows the current playback position and allows to do some fast scrubbing is no longer being updated when the movie is playing. The bug is related to the styling of the thumb. If the thumb is not styled, it is correctly updated when the value is changed by script. If the thumb is styled, it does not move anymore when the value is changed. To reproduce: - Go to http://www.w3schools.com/html5/tryit.asp?filename=tryhtml5_form_range - Enter the code below in the left frame - Click "Edit and Click Me >>" ==> BUG: The custom-styled thumb does not move every second as it should. - Then remove the style rule that matches the thumb ("input[type="range"]::-webkit-slider-thumb") - Click "Edit and Click Me >>" ==> OK: The native-looking thumb moves when the value is updated by script. - Restore the style rule and re-activate the commented code that toggles between "inline-block" and "inline" - Click "Edit and Click Me >>" ==> WORKAROUND: The custom-styled thumb now moves correctly (but that's quite an ugly workaround) CCd dglazkov who recently refactored the slider. Here is the code: ======== <!DOCTYPE HTML> <html> <style> input[type="range"] { -webkit-appearance: none; height: 10px; border-radius: 4px; background: -webkit-gradient(linear, 0% 0%, 0% 100%, from(#444), color-stop(0.4, #888), color-stop(0.6, #888), to(#444)); } input[type="range"]::-webkit-slider-thumb { -webkit-appearance: none; width: 10px; height: 28px; border-radius: 4px; background: -webkit-gradient(linear, 0% 0%, 100% 0%, from(#222), color-stop(0.4, #888), color-stop(0.6, #888), to(#222)); } </style> <body> Points: <input id="slider" type="range" min="0" max="10" /> <input type="submit" /> <script> var slider = document.getElementById('slider'); slider.value = 0; setTimeout("test()", 1000); function test() { var slider = document.getElementById('slider'); slider.value++; //if (!(slider.value % 2)) //setTimeout("slider.style.display = 'inline-block'", 1); //else //setTimeout("slider.style.display = 'inline'", 1); if (slider.value < 9) setTimeout("test()", 1000); } </script> </body> </html>
Attachments
test case (1.01 KB, text/html)
2011-03-10 11:11 PST, Alexey Proskuryakov 		no flags
Details
Patch (9.78 KB, patch)
2011-03-15 15:37 PDT, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2011-03-10 11:09:20 PST
Confirmed as a regression from Safari 5 using a local build of r80599.
Alexey Proskuryakov
Comment 2 2011-03-10 11:09:38 PST
<rdar://problem/9115880>
Alexey Proskuryakov
Comment 3 2011-03-10 11:11:05 PST
Created attachment 85351 [details] test case Same test as an attachment.
Dimitri Glazkov (Google)
Comment 4 2011-03-10 12:50:45 PST
Fixing...
Dimitri Glazkov (Google)
Comment 5 2011-03-14 16:48:38 PDT
This is more complex than I anticipated...
Dimitri Glazkov (Google)
Comment 6 2011-03-15 15:37:28 PDT
Created attachment 85865 [details] Patch
Kent Tamura
Comment 7 2011-03-15 17:55:32 PDT
Comment on attachment 85865 [details] Patch ok
WebKit Commit Bot
Comment 8 2011-03-15 21:09:55 PDT
The commit-queue encountered the following flaky tests while processing attachment 85865 [details]: java/lc3/JSObject/ToObject-001.html bug 53091 (author: ap@webkit.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 9 2011-03-15 21:14:06 PDT
Comment on attachment 85865 [details] Patch Clearing flags on attachment: 85865 Committed r81216: <http://trac.webkit.org/changeset/81216>
WebKit Commit Bot
Comment 10 2011-03-15 21:14:10 PDT
All reviewed patches have been landed. Closing bug.
piet
Comment 11 2011-03-16 10:52:48 PDT
If I may make a minor comment... I think that the bulk of SliderThumbElement::setPositionFromPoint() should be moved into a method called ::setValueFromPoint() and at the very end of the function, it should call ::setPositionFromValue() instead of calling directly renderer()->setNeedsLayout(true). So it should look something like: void SliderThumbElement::setPositionFromPoint(const IntPoint& point) { HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowHost()); ASSERT(input); if (!input->renderer() || !renderer()) return; setValueFromPoint(point); setPositionFromValue(); input->dispatchFormControlChangeEvent(); } It describes better what happens. Reopen if you like.
Dimitri Glazkov (Google)
Comment 12 2011-03-16 11:21:30 PDT
(In reply to comment #11) > If I may make a minor comment... I think that the bulk of SliderThumbElement::setPositionFromPoint() should be moved into a method called ::setValueFromPoint() and at the very end of the function, it should call ::setPositionFromValue() instead of calling directly renderer()->setNeedsLayout(true). > > So it should look something like: > > void SliderThumbElement::setPositionFromPoint(const IntPoint& point) > { > HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowHost()); > ASSERT(input); > > if (!input->renderer() || !renderer()) > return; > > setValueFromPoint(point); > setPositionFromValue(); > input->dispatchFormControlChangeEvent(); > } > > It describes better what happens. Reopen if you like. Sounds like a great first patch! :)
Note You need to log in before you can comment on or make changes to this bug.