Summary: | MediaControlTimelineElement is adjusting time 3 times per click | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dimitri Glazkov (Google) <dglazkov> | ||||||
Component: | Media | Assignee: | Jer Noble <jer.noble> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | arun.patole, eric.carlson, feature-media-reviews, jer.noble, webkit.review.bot | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Dimitri Glazkov (Google)
2011-04-08 13:42:28 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?? (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. Dimitri, I'll take this one. Created attachment 143874 [details]
Patch
Comment on attachment 143874 [details]
Patch
Nope, this patch breaks dragging the timeline slider.
Hey, whaddaya know. There's already an "input" message which is fired on input elements during mousedown and mousemove events. Created attachment 143879 [details]
Patch
Committed r118411: <http://trac.webkit.org/changeset/118411> |