Closed Bug 475286 Opened 16 years ago Closed 16 years ago

Video scrubber should jump to clicked position, not take incremental steps

Categories

(Toolkit :: Video/Audio Controls, enhancement)

enhancement
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: BijuMailList, Assigned: enndeakin)

References

Details

(Keywords: verified1.9.1)

Attachments

(1 file, 1 obsolete file)

Bug 462113 implemented progress bar / scrubber for video controls Seeking in current implementation is not intuitive enough. Or in other words it dont behave like familiar video players like Youtube, Win Media Player, Quicktime etc. ie, clicking on the "Track" do not seek the current position to point of clicking. instead user has to keep holding mouse button down for a while to make "scroll thumb" slowly come to the mouse position. (PS: dragging the thumb works fine) Steps:- 1. open a page with video (http://www.double.co.nz/video_test/test2.html) 2. enable controls 3. play video 4. wait few seconds 5. click on progress bar Result:- nothing happens Expected:- Video current position moved to clicked position
I don't think it's not intuitive, I've used plenty of video players with the same issue (granted, years ago). But, I wouldn't say this is a blocker anymore than not having a volume level control.
(In reply to comment #1) > But, I wouldn't say this is a blocker anymore > than not having a volume level control. Yes we definitely need volume level control it was there in the mockup http://jboriss.wordpress.com/2008/09/19/html-5-video-tag-pirate-edition/ I have created bug 475317
This was partially fixed by bug 475861.
(In reply to comment #3) > This was partially fixed by bug 475861. Far better now... But still why the thumb dont come to clicked position immediately. I feel this is because bug 475441 see bug 475441 comment 7 Case 2 and 3 there the thumb comes to clicked position immediately.
Every major modern media player out there seeks to where you click on the progress bar, rather than advancing the currentTime by some fraction of the duration of the video, which is what we do. Our interface should match what all the other players do, so that our behaviour is expected, and thus intuitive. By seeking to progress bar click position on click, we'd be matching seek behaviour with the following players: * Youtube * Vimeo * Yahoo Video * MSN Video * Windows Media Player * Windows Media Player browser plugin * Quicktime Player * iTunes * Winamp * VLC Player
Flags: wanted1.9.1?
Flags: blocking1.9.1?
The current implementation basically only does this because that's how a XUL <scale> behaves. I agree this should change. We'll probably need to add an attribute to <scale> to allow selecting what a click does, but maybe it can be done by extending the binding.
Assignee: nobody → dolske
Summary: Seeking video by clicking on progress bar is not intuitive enough. → Video scrubber should jump to clicked position, not take incremental steps
I agree that this feels important for the success of <video> and should probably block as it's expected behaviour. Neil: can you get in touch with Dolske and lend a hand?
Flags: blocking1.9.1? → blocking1.9.1+
This should be an easy fix as the functionality is already present. Holding down Shift (Option on Mac) and clicking should be the desired behaviour. I think scales just want this as the default instead.
Attached patch implement this (obsolete) (deleted) — Splinter Review
This patch implements a movetoclick attribute for <scale> which adds click to move to that point. On Mac, the default value is true, but on other platforms the default is false. This matches the behaviour of generic native scale widgets in control panels and the like. The videocontrols has movetoclick="true" which gives this behaviour on all platforms.
Assignee: dolske → enndeakin
Status: NEW → ASSIGNED
Attachment #368087 - Flags: superreview?(neil)
Attachment #368087 - Flags: review?(neil)
Comment on attachment 368087 [details] [diff] [review] implement this >+ // if there is no parent scrollbar, check the movetoclick attribute. If set >+ // to true, always scroll to the click point. If false, never do this. >+ // Otherwise, the default is true on Mac and false on other platforms. Let me get this straight. For sliders, you want to ignore the scroll to click look and feel preference on the Mac, instead just defaulting to true? > // Check if we should scroll-to-click regardless of the pressed button and > // modifiers > if (!scrollToClick) >+#endif > scrollToClick = GetScrollToClick(); >-#endif This looks totally bogus anyway because it was in an #ifndef XP_MACOSX block, but I'm worried that this change will mean that an unshifted thumb click will now scroll to click on the Mac too, which I think would be wrong.
I don't think it's actually needed. Non-Mac only checks the scroll-to-click setting currently in this one case which doesn't seem useful on its own.
Attachment #368087 - Attachment is obsolete: true
Attachment #369703 - Flags: superreview?(neil)
Attachment #369703 - Flags: review?(neil)
Attachment #368087 - Flags: superreview?(neil)
Attachment #368087 - Flags: review?(neil)
Attachment #369703 - Flags: superreview?(neil)
Attachment #369703 - Flags: superreview+
Attachment #369703 - Flags: review?(neil)
Attachment #369703 - Flags: review+
Comment on attachment 369703 [details] [diff] [review] ok, just remove the GetScrollToClick code there. >-// Helper function to collect the "scroll to click" metric. Beware of >-// caching this, users expect to be able to change the system preference >-// and see the browser change its behavior immediately. >-static PRBool >-GetScrollToClick() Is it not possible to keep the function at the same place in the file? >+ // if there is no parent scrollbar, check the movetoclick attribute. If set >+ // to true, always scroll to the click point. If false, never do this. >+ // Otherwise, the default is true on Mac and false on other platforms. >+ if (GetScrollbar() == this) >+#ifdef XP_MACOSX >+ return !mContent->AttrValueIs(kNameSpaceID_None, nsGkAtoms::movetoclick, >+ nsGkAtoms::_false, eCaseMatters); This is very confusingly written, but I guess that's always going to happen when the behaviours are so different between Mac and non-Mac.
> Is it not possible to keep the function at the same place in the file? Could leave it as is, but it seems odd for that method to be with the static declarations before the constructor.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Enn: can we get this landed on 1.9.1 for Beta 4?
Keywords: fixed1.9.1
Please include the changeset in the comments next time, thanks. Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b5pre) Gecko/20090507 Shiretoko/3.5b5pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090507 Minefield/3.6a1pre
Status: RESOLVED → VERIFIED
Flags: in-litmus+
Component: Video/Audio → Video/Audio Controls
Product: Core → Toolkit
QA Contact: video.audio → video.audio
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: