The code in for dealing with timeupdate events in videocontrols.xml#handleEvent does not move the scrubber if the media's currentTime has changed by less than 333ms since the last timeupdate event. This can result in the scrubber's last position update being quite far from the end of the buffer bar when playing back short files--this is most obvious with Wave files. See the file linked in the URL for an example. I think the ended event should move the scrubber to the end of the buffer bar. (I originally thought of making the code to discard frequent timeupdate events handle the special case of currentTime being within an epsilon of the duration and allowing the scrubber to move in that case. But we might not know the duration, so we can't do that.)

So, this seems like the obvious fix... but after the scrubber reaches the end, something causes it to reset to the beginning.

Taking this to get it on my radar for 3.5, but feel free to steal back if you really want to. :)

Easy to see the problem with this clip, it's short but wide, and the thumb usually stops a dozen pixels or so from the end.

(In reply to comment #1) > but after the scrubber reaches the end, > something causes it to reset to the beginning. Two (!) bugs: I've found that on some videos, the time index of the last frame is -1. Seems like a core bug, it shouldn't ever be < 0. Additionally, your patch was feeding the raw currentTime value to showPosition(), when it actually expects milliseconds. So that would always make it jump to nearly the beginning.

Seems like a pretty visible problem given how easy it is to reproduce in bug 493523

(In reply to comment #5) > Two (!) bugs: I've found that on some videos, the time index of the last frame > is -1. Seems like a core bug, it shouldn't ever be < 0. Probably bug 493431 for which I just landed a patch.

Wouldn't block release on this, would still like a patch.

Updated patch, no change, just fix merge conflict with bug 492213 which will likely land before this.

Comment on attachment 378478 [details] [diff] [review] Patch v.2 Since showPosition is always called with the same parameters (which aren't used elsewhere in the calling code), it seems to me like it would make the most sense to factor that code out a bit more and just have a showPosition() that takes a single argument indicating whether or not it should throttle based on the last lastTimeUpdate.

Comment on attachment 378478 [details] [diff] [review] Patch v.2 a191=beltzner Won't hold release for this, but should be super-safe. Make sure it bakes for a cycle on mozilla-central before you move it to 1.9.1 File a follow-up for tests?

Pushed http://hg.mozilla.org/mozilla-central/rev/051f97ebed89

Pushed to 191: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/59d2e9c32a81

verified FIXED on builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090527 Minefield/3.6a1pre ID:20090527031500 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090527 Shiretoko/3.5pre ID:20090527031214