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)
Toolkit
Video/Audio Controls
Tracking
()
VERIFIED
FIXED
People
(Reporter: BijuMailList, Assigned: enndeakin)
References
Details
(Keywords: verified1.9.1)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
neil
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
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
Comment 3•16 years ago
|
||
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.
Comment 5•16 years ago
|
||
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?
Comment 6•16 years ago
|
||
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
Comment 7•16 years ago
|
||
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+
Assignee | ||
Comment 8•16 years ago
|
||
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.
Assignee | ||
Comment 9•16 years ago
|
||
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 10•16 years ago
|
||
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.
Flags: wanted1.9.1?
Assignee | ||
Comment 11•16 years ago
|
||
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)
Updated•16 years ago
|
Attachment #369703 -
Flags: superreview?(neil)
Attachment #369703 -
Flags: superreview+
Attachment #369703 -
Flags: review?(neil)
Attachment #369703 -
Flags: review+
Comment 12•16 years ago
|
||
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.
Assignee | ||
Comment 13•16 years ago
|
||
> 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.
Assignee | ||
Updated•16 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 14•16 years ago
|
||
Enn: can we get this landed on 1.9.1 for Beta 4?
Assignee | ||
Updated•16 years ago
|
Keywords: fixed1.9.1
Comment 15•16 years ago
|
||
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
Keywords: fixed1.9.1 → verified1.9.1
Comment 16•16 years ago
|
||
Added testcase to Fx3.5 FFT: https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=7690
Flags: in-litmus+
Updated•15 years ago
|
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.
Description
•