RESOLVED FIXED58160
MediaControlTimelineElement is adjusting time 3 times per click 
https://bugs.webkit.org/show_bug.cgi?id=58160
Summary MediaControlTimelineElement is adjusting time 3 times per click
Dimitri Glazkov (Google)
Reported 2011-04-08 13:42:28 PDT
The defaultEventHandler of the class does not distinguish between mouseup/down/click, thus setting currentTime thrice on each click. This seems suboptimal.
Attachments
Patch (1.81 KB, patch)
2012-05-24 13:11 PDT, Jer Noble 		no flags
DetailsFormatted DiffDiff
Patch (1.76 KB, patch)
2012-05-24 13:28 PDT, Jer Noble 		eric.carlson: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Arun Patole
Comment 1 2011-10-02 23:21:09 PDT
(In reply to comment #0) > The defaultEventHandler of the class does not distinguish between mouseup/down/click, thus setting currentTime thrice on each click. This seems suboptimal. just had a look at the code, looks like once currentime is set first time, condition "if (time != mediaElement()->currentTime())" will fail next time as time is already adjusted so won't adjust the time again??
Jer Noble
Comment 2 2012-05-24 12:54:16 PDT
(In reply to comment #1) > just had a look at the code, looks like once currentime is set first time, condition "if (time != mediaElement()->currentTime())" will fail next time as time is already adjusted so won't adjust the time again?? Not necessarily. For some media engines, a seek is asynchronous, and the end result of a seek may not be exactly the time requested. Three calls in a row to seek() could definitely occur. Sounds like setCurrentTime() should only occur for mousedown and mousemove events.
Jer Noble
Comment 3 2012-05-24 12:54:34 PDT
Dimitri, I'll take this one.
Jer Noble
Comment 4 2012-05-24 13:11:35 PDT
Created attachment 143874 [details] Patch
Jer Noble
Comment 5 2012-05-24 13:14:18 PDT
Comment on attachment 143874 [details] Patch Nope, this patch breaks dragging the timeline slider.
Jer Noble
Comment 6 2012-05-24 13:27:31 PDT
Hey, whaddaya know. There's already an "input" message which is fired on input elements during mousedown and mousemove events.
Jer Noble
Comment 7 2012-05-24 13:28:28 PDT
<rdar://problem/11350416>
Jer Noble
Comment 8 2012-05-24 13:28:48 PDT
Created attachment 143879 [details] Patch
Jer Noble
Comment 9 2012-05-24 13:41:08 PDT
Committed r118411: <http://trac.webkit.org/changeset/118411>
Note You need to log in before you can comment on or make changes to this bug.