Closed
Bug 1239372
Opened 9 years ago
Closed 8 years ago
The tab sound indicator should not be hiding when using mouse clicking to seek the video
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 50
Tracking | Status | |
---|---|---|
firefox50 | --- | verified |
People
(Reporter: alwu, Assigned: alwu)
References
Details
Attachments
(1 file)
STR.
1. Go to https://www.youtube.com/watch?v=cxeQmF4sUJY
2. Seek to arbitrary position
Expected.
3. Sound icon shouldn't disappear during seeking
Actual.
3. Sound icon disappear during seeking
Assignee | ||
Comment 1•9 years ago
|
||
In this case, this process would trigger MediaElement::Pause() before the MediaElement::Seek(), so the problem happen.
However, this behavior isn't equal to every MediaElement. I tried to seek another audio element [1], and I found that the MediaElement::Seek() was triggered before MediaElement::Seek().
Here are two things we need to get them more clear,
First, do we need to call Pause() when seeking the video?
Second, what's the correct behavior when seeking the MediaElement?
[1] http://people.mozilla.org/~alwu/Test.html
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → alwu
1) Your Step 2 isn't precise enough. Does "seek" means "click on progressbar" or "hover mouse over
progressbar, hold left mouse button, then move mouse pointer within progressbar, then
release left mouse button"? Or maybe "Hold Right arrow on the keyboard"?
Actually, in all 3 cases producing sound is undesired option and therefore
2) The "Actual" result in comments 0-1 is completely expected, since nothing is causing sound. It's
not clear why you're calling a problem the sound indicator being hidden when there's no sound
See also bug 1225000
Assignee | ||
Comment 3•9 years ago
|
||
Hi, Arni,
In this issue, the case I want to discuss here is "click on progressbar". The sound icon should only disappear when the second case you mentioned happen (hover mouse over progressbar...) Another interesting point is the third case (Hold Right arrow on the keyboard) doesn't let the sound icon disappear, and it's exactly the ideal behavior in my mind.
However, IMMO, the sound icon shouldn't disappear in rest of these cases. Seeking is a transition behavior, it happens during very short period, we should keep the sound icon state as same as the state before seeking.
If you think the sound icon need to disappear during this very short period, that means the users would see the icon blinking when they watch the video with a lots of short silent. In addition, the UX suggest is in [1]. It said that we only need to handle the silence longer than 10 seconds. The bug 1225000 should also follow the 10-seconds rule.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1235612#c18
Flags: needinfo?(arni2033)
Assignee | ||
Comment 4•9 years ago
|
||
Hi, Philipp,
Sorry to bother you, need your help again!
Could you help me verify whether the sound icon should be displayed or not in the following situations?
* pre-condition : Users are watching an audible video, and then they want to seek video *
Here are three different kinds of seeking.
(1) Click on the specific position on the progress-bar, they want the video start playing from that
position.
(2) Hover mouse over progress-bar, hold left mouse button, then move mouse pointer within progress-bar,
then user can find the position they want from the preview image, release left mouse button until they
decide where the video should start playing from.
(3) Hold left/right arrow on the keyboard, to decrease/increase a fixed-interval for position.
Very appreciate :)
Flags: needinfo?(philipp)
I see now, you only want to reduce blinking in this bug. However, messing with timeouts doesn't look like a good idea: If in scenario (2) I move mouse about 10.1-10.5 seconds and then release left mouse button, then I will still see blinking "visible -> invisible -> visible"
Also, this was already requested in bug 1232357. That's really ~5 lines of CSS to make a transition.
Anyway, here's a few notes about comments 3-4:
(1) in comment 4 is subset of (2), so this somehow reduces the number of cases...
Scenario (3) actually doesn't hide the sound indicator, yes. That's because it doesn't stop the video!
And sometimes I hear some short sounds during seeking. Video/audio element can't tell the difference between "press" and "hold", and that sounds like a separate bug (enhancement?) to me.
Flags: needinfo?(arni2033)
Assignee | ||
Comment 6•9 years ago
|
||
I like the idea of the bug1232357, if we can do this kinds of fading animation that we wouldn't worry about the icon blinking.
If so, I think this issue should be solved in the front-end side, we would still dispatch the "audio-playback-false" event when the audio pausing or no any audible sound. However, the front-end side need to handle the logic about icon disappearing (or fading).
Comment 7•9 years ago
|
||
(In reply to Alastor Wu [:alwu] from comment #4)
> Hi, Philipp,
> Sorry to bother you, need your help again!
> Could you help me verify whether the sound icon should be displayed or not
> in the following situations?
>
> * pre-condition : Users are watching an audible video, and then they want to
> seek video *
>
> Here are three different kinds of seeking.
> (1) Click on the specific position on the progress-bar, they want the video
> start playing from that
> position.
> (2) Hover mouse over progress-bar, hold left mouse button, then move mouse
> pointer within progress-bar,
> then user can find the position they want from the preview image,
> release left mouse button until they
> decide where the video should start playing from.
> (3) Hold left/right arrow on the keyboard, to decrease/increase a
> fixed-interval for position.
>
> Very appreciate :)
Hm, if I understand you correctly, all of the scenarios depend upon whether or not sound is played during seeking. If the page keeps playing audio while seeking: keep showing the indicator. If not: wait for the timeout and if it still isn't playing audio after that: hide it.
I also like the idea of fading the indicator in and out. That would make things a lot smoother. It might not be possible though, because the indicator is changing the size of the tab label, and I don't think we can animate that easily.
Flags: needinfo?(philipp)
Assignee | ||
Comment 8•9 years ago
|
||
I would modify our native video-control, let the sound icon keeps showing during the seeking with clicking.
So the STR is like following,
STR.
1. Go to http://people.mozilla.org/~alwu/Test.html
2. Play video
3. Use mouse to click on the specific position on the progress-bar, and then release mouse
Expected.
4. Sound icon shouldn't disappear during seeking
Actual.
4. Sound icon disappear during seeking
Summary: [Tab sound indicator] The sound icon would disappear when seeking the video → The tab sound indicator should not be hiding when using mouse clicking to seek the video
Assignee | ||
Comment 9•9 years ago
|
||
For youtube, they have their own video-control-interface, so we can't control it.
Assignee | ||
Comment 10•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/35231/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35231/
Assignee | ||
Comment 11•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8720215 -
Flags: review?(jaws)
Assignee | ||
Comment 12•9 years ago
|
||
Hi, Jared,
Sorry to bother you, could you help me review this patch?
Very appreciate :)
---
The main idea of this patch is to prevent the tab sound indicator flickering when users use clicking to seek the video. The reproduce steps is in the comment8.
I also list the possible seeking behaviors in comment4, we only need to call video.pause in the case (2), when user is continually dragging the progress-bar and doens't trigger mouse-up.
Comment 13•9 years ago
|
||
Hi Alastor, thanks for the patch. Fixing it in the video controls will only make the experience better for HTML5 media that uses our default controls, but not sites like YouTube that provide their own controls.
A better fix would be bug 1232357 which will work everywhere and should cover the case of seeking within a video.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Updated•9 years ago
|
Attachment #8720215 -
Flags: review?(jaws)
Assignee | ||
Comment 14•9 years ago
|
||
Ok, thanks :)
Assignee | ||
Comment 15•8 years ago
|
||
Hi, Jared,
I think we still need that because on Fennec we use the default control interface, the media control [1] would be flashing if we don't fix this issue. The bug 1232357 only affects on the desktop.
How do you think?
Thanks!
Status: RESOLVED → REOPENED
Flags: needinfo?(jaws)
Resolution: DUPLICATE → ---
Assignee | ||
Comment 16•8 years ago
|
||
FYI, Youtube uses our default control interface on Fennec.
Comment 17•8 years ago
|
||
I didn't think of that before. I'm a bit worried about not pausing during dragging and what other types of bugs we may introduce due to the amount of state that we store in the videocontrols binding.
I think a more robust fix, and one that would also fix bug 1232357 would be to change the lower level code that announces audio has stopped. That code should have the 3 second delay built in to it.
Flags: needinfo?(jaws)
Assignee | ||
Comment 18•8 years ago
|
||
Hi, Jaren,
Sorry for the late reply.
Could you help me explain the potential side affect if we do seeking before pausing?
IMMO, the platform audio notification should be reflected the audio playing state precisely. If we want to do some delay, it might be implemented in front-end code for each different platform.
Eg. The media control interface on Fennec is also depend on the platform audio notification to decide when it should be show/hide. And I don't want to delay the hiding time for that.
Thanks!
Flags: needinfo?(jaws)
Updated•8 years ago
|
Attachment #8720215 -
Flags: review+
Comment 19•8 years ago
|
||
Comment on attachment 8720215 [details]
Bug 1239372 - only pause video during draging.
https://reviewboard.mozilla.org/r/35231/#review56302
Okay, let's go with this for now and see what comes out of it. The code looks okay and not too different than what we were doing before besides the behavioral change.
Updated•8 years ago
|
Flags: needinfo?(jaws)
Comment 20•8 years ago
|
||
(In reply to Alastor Wu [:alwu] from comment #18)
> Hi, Jaren,
> Sorry for the late reply.
> Could you help me explain the potential side affect if we do seeking before
> pausing?
We have had bugs before about the videocontrols making some assumptions about the internal state of the video due to some other state being cached by the videocontrols. I'm not sure if this will cause any of those but we can try this out to see. It looks sound to me.
> IMMO, the platform audio notification should be reflected the audio playing
> state precisely. If we want to do some delay, it might be implemented in
> front-end code for each different platform.
>
> Eg. The media control interface on Fennec is also depend on the platform
> audio notification to decide when it should be show/hide. And I don't want
> to delay the hiding time for that.
Yes, you are right. We shouldn't lie about when the audio stops. The delay should remain a front-end feature.
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8720215 [details]
Bug 1239372 - only pause video during draging.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35231/diff/1-2/
Attachment #8720215 -
Attachment description: MozReview Request: Bug 1239372 - only pause video during draging. → Bug 1239372 - only pause video during draging.
Assignee | ||
Comment 22•8 years ago
|
||
Comment 23•8 years ago
|
||
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cef62921024
only pause video during draging. r=jaws
Comment 24•8 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 9 years ago → 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Comment 25•8 years ago
|
||
I have reproduced this bug with Firefox Nightly 46.0a1 (Build ID:20160113030208) on
Windows 8.1, 64-bit.
Verified as fixed with Firefox Nightly 50.0a1 (Build ID: 20160713030216)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
QA Whiteboard: [bugday-20160713]
Comment 26•8 years ago
|
||
Verified as fixed on Firefox Aurora 50.0a2(2016-09-09) using Linux Mint 18.
Comment 27•8 years ago
|
||
[testday-20160909]
Comment 28•8 years ago
|
||
Verified on FF 50.3beta
[testday-20160930]
Comment 29•8 years ago
|
||
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•