Open Bug 1352995 Opened 7 years ago Updated 2 years ago

Play button should not stop working unless media source is fully detached

Categories

(Toolkit :: Video/Audio Controls, defect, P3)

52 Branch
defect

Tracking

()

Tracking Status
firefox52 --- wontfix
firefox53 --- affected
firefox54 --- affected
firefox55 --- affected

People

(Reporter: ralin, Unassigned)

References

Details

After bug 1302656 landed, video might continue playing even network error occurred. User need to be able to pause and play video if media source isn't detached.
See Also: → 1346432
Could you please be a bit more descriptive about what the problem is, what it should be and what it actually does?

Right now, I don't understand your bug description...

Being able to play or not has nothing to do with the media source object being attached or not following an error.
Sorry about that. 

Let me copy the STR from bug 1346432 comment #2:

STR:
0. Run ./mach mozregression -g dbbd4d6ee7ec -b d392ffb70683 to compare before/after bug 1302656 landed
------
1. Install and enable https://addons.mozilla.org/en-US/firefox/addon/inlinedisposition/
2. Open https://www.ssyoutube.com/watch?v=ZGqhhx8cxMg , Click "Download" button on the page
3. Pause the video, wait until significant part of the video is cached, at least 10% of the video, but no more than 50%. Cached part of the video is displayed as a lightgray bar in timeline (timline is gray horizontal bar)
4. Play the video
5. Disable internet connection
6. Wait until the video displays network error


AR: video continue playing, but clicking on play(pause) button does nothing. 
ER: video continue playing, but clicking on play(pause) button should be able to pause video. 

Regression window: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=060f80b690b8aaa5d927e03578673a3eff3b4c64&tochange=000dc91517d648344729bc8764aee1cca8e91e77


------
Before bug 1302656 landed, media is no more playable when error occurred as decoder will be shutdown, so it's sensible for play button to do nothing[0].
After the change[1], media could be playable even error occurred, then play button should be workable at that point.

[0] https://dxr.mozilla.org/mozilla-central/rev/38894655c89e68bcd8f45d31a0d3005f2c2b53db/toolkit/content/widgets/videocontrols.xml#1282-1290
[1] https://hg.mozilla.org/mozilla-central/rev/d392ffb70683#l1.32
Then the title is wrong, because no media source (MSE) is involved with https://www.ssyoutube.com

Shouldn't an error (decode or network) be fatal?
Flags: needinfo?(jwwang)
https://html.spec.whatwg.org/multipage/embedded-content.html#dom-media-play
2. If the media element's error attribute is not null and its code is MEDIA_ERR_SRC_NOT_SUPPORTED, return a promise rejected with a "NotSupportedError" DOMException and abort these steps.

It looks like only MEDIA_ERR_SRC_NOT_SUPPORTED will prevent playback from starting.
Flags: needinfo?(jwwang)
Sure, but why would play/pause have no effect then?
(In reply to Ray Lin[:ralin] from comment #2)
> Before bug 1302656 landed, media is no more playable when error occurred as
> decoder will be shutdown, so it's sensible for play button to do nothing[0].
> After the change[1], media could be playable even error occurred, then play
> button should be workable at that point.
> 
> [0]
> https://dxr.mozilla.org/mozilla-central/rev/
> 38894655c89e68bcd8f45d31a0d3005f2c2b53db/toolkit/content/widgets/
> videocontrols.xml#1282-1290
> [1] https://hg.mozilla.org/mozilla-central/rev/d392ffb70683#l1.32

Do you mean we should change the video control so the play button is effective even when an error is encountered?
Flags: needinfo?(ralin)
well, that's what this bug is about...

when you encounter a error (not MEDIA_ERR_SRC_NOT_SUPPORTED) then playback continues but you can't stop it as play is non-functional!
Thanks for clarification, :jya

I guess, for media controls play button handler[0], we would need to early return only if MEDIA_ERR_SRC_NOT_SUPPORTED occurred, otherwise, play button should works as-is.

[0] https://dxr.mozilla.org/mozilla-central/rev/720b9177c6856c1c4339d0fac1bf5149c0d53950/toolkit/content/widgets/videocontrols.xml#1278-1287
Flags: needinfo?(ralin)
Assignee: nobody → ralin
Status: NEW → ASSIGNED
Assignee: ralin → nobody
Status: ASSIGNED → NEW
Severity: normal → S3
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.