Closed Bug 1420488 Opened 2 years ago Closed 2 years ago

YouTube video (but not audio) stops playing on tab change when the pref "media.autoplay.enabled" is false

Categories

(Core :: Audio/Video: Playback, defect, P3, major)

defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- unaffected
firefox59 --- fixed

People

(Reporter: johnp, Assigned: alwu)

References

Details

(Keywords: regression)

Attachments

(2 files)

Since bug 1382574 (mozregression range https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ab0a609b4db5a5e9d99f48c37106f850ff4f2e6a&tochange=356b50c5becf41a2839ecb895e7c2219223f55b5) video stops playing on youtube after switching to other tabs. The audio keeps playing and the timestamp keeps increasing, but the control buttons all reset to the "start playing" state and instead of the video, it's thumbnail is shown. Sometimes this also happens without changing tabs, but just by scrolling on the page so that the video is outside of the viewport.

I ran mozregression on my main profile, so I'm not sure if `media.autoplay.enabled=false` is enough to trigger this bug. I'll check a fresh profile as well and report back.
Verified the regression range with a fresh profile and only `media.autoplay.enabled=false`; resizing the video to theater mode after starting it with the bottom left play button is enough to trigger the bug as well.
Flags: needinfo?(alwu)
Keywords: regression
Priority: -- → P3
Hi, Johannes,
So you mean your video freeze but audio is still playing after switching tab or scrolling video outside the viewport?
And also, UI would change from "pause button" to "play button" after issue happen?

BTW, I can't reproduce this issue on OSX 10.11.4.
Assignee: nobody → alwu
Flags: needinfo?(alwu) → needinfo?(johnp)
That's right. I confirm the issue. 

Windows 7 here.
Ok, I can reproduce it on win7, I'll investigate it.
For a temporary workaround, keep the pref "media.autoplay.enabled" off and turn on the pref "media.autoplay.enabled.user-gestures-needed" which is a new autoplay policy which is slightly different with the present behavior.
The video doesn't freeze, but resets to the usual preview image with the big play button in the middle. Everything else is as you described.

Linux x86_64 here.
Flags: needinfo?(johnp)
Summary: YouTube video (but not audio) stops playing on tab change → YouTube video (but not audio) stops playing on tab change when the pref "media.autoplay.enabled" is false
Comment on attachment 8932383 [details]
Bug 1420488 - part1 : bless media if media has started playing before.

https://reviewboard.mozilla.org/r/203402/#review209236

::: commit-message-6bb6f:3
(Diff revision 1)
> +Bug 1420488 - part1 : allow play() if media has been played.
> +
> +If user calls video.play() and video is playing now, then we won't reject the play promise.

Can you explain why the 1st play() call is not rejected by AutoplayPolicy::IsMediaElementAllowedToPlay() as the 2nd call?
(In reply to JW Wang [:jwwang] [:jw_wang] from comment #8)
> Can you explain why the 1st play() call is not rejected by
> AutoplayPolicy::IsMediaElementAllowedToPlay() as the 2nd call?

If the pref "media.autoplay.enabled.user-gestures-needed" is enable, the play() would be allowed after the page has been activated by user.

If the pref is off, the play() would be allowed if it's triggered by user input.
I don't follow.

Can you elaborate the scenario of the issue?
(In reply to JW Wang [:jwwang] [:jw_wang] from comment #12)
> I don't follow.
> 
> Can you elaborate the scenario of the issue?

The case here is 
1. user click-to-play the video on youtube 
   - the play() is triggered by user-input, allow to play
2. video starts playing
3. youtube calls play() again when the video is still playing
   - since the video is playing, so we should not reject the promise
(In reply to Alastor Wu [:alwu][please needinfo me][GMT+8] from comment #13)
> The case here is 
> 1. user click-to-play the video on youtube 
>    - the play() is triggered by user-input, allow to play
> 2. video starts playing
> 3. youtube calls play() again when the video is still playing
>    - since the video is playing, so we should not reject the promise

What if step 3 calls play() when playback has reached the end and paused? Will it reject the play promise?
(In reply to JW Wang [:jwwang] [:jw_wang] from comment #14)
> What if step 3 calls play() when playback has reached the end and paused?
> Will it reject the play promise?

If the playback is ended, it's same as non-start case. As I said in comment9.
Comment on attachment 8932383 [details]
Bug 1420488 - part1 : bless media if media has started playing before.

https://reviewboard.mozilla.org/r/203402/#review209314

::: dom/html/HTMLMediaElement.h:1788
(Diff revision 3)
>    // Total time a video has (or would have) spent in video-decode-suspend mode.
>    TimeDurationAccumulator mVideoDecodeSuspendTime;
>  
> -  // True if user has called load() or seek() via user input.
> +  // True if user has called load(), seek() or element has started playing before.
>    // It's *only* use for checking autoplay policy
> -  bool mHasUserInteractedLoadOrSeek;
> +  bool mIsBlessed;

Use in-class initialization.

bool mIsBlessed = false;
Attachment #8932383 - Flags: review?(jwwang) → review+
Comment on attachment 8932384 [details]
Bug 1420488 - part2 : add test.

https://reviewboard.mozilla.org/r/203430/#review209322

::: toolkit/content/tests/browser/browser_autoplay_policy_play_twice.js:27
(Diff revision 3)
> +
> +  info("- simulate user-click to start video -");
> +  let playPromise = once(video, "play");
> +  await BrowserTestUtils.synthesizeMouseAtCenter(video, {button: 0},
> +                                                 tab.linkedBrowser);
> +  await playPromise;

This should be wrapped in try/catch as below.

::: toolkit/content/tests/browser/browser_autoplay_policy_play_twice.js:28
(Diff revision 3)
> +  info("- simulate user-click to start video -");
> +  let playPromise = once(video, "play");
> +  await BrowserTestUtils.synthesizeMouseAtCenter(video, {button: 0},
> +                                                 tab.linkedBrowser);
> +  await playPromise;
> +  ok(!video.paused, "video starts playing");

It should be sufficient to just check whether the play promise is resolved or rejected.

::: toolkit/content/tests/browser/browser_autoplay_policy_play_twice.js:33
(Diff revision 3)
> +  ok(!video.paused, "video starts playing");
> +
> +  info("- call video play() again -");
> +  try {
> +    await video.play();
> +    ok(!video.paused, "play() success");

Ditto. If play() succeeds, no expection will be thrown.
Attachment #8932384 - Flags: review?(jwwang) → review+
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0f211029517c
part1 : bless media if media has started playing before. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/92bd0fcb67d5
part2 : add test. r=jwwang
Backed out for eslint failures in toolkit/content/tests/browser/browser_autoplay_policy_play_twice.js:

https://hg.mozilla.org/integration/autoland/rev/6cbc9082c4352078036b20f7fff27f93c13fcdf0

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=92bd0fcb67d58bcd952d5d2930381f40e1d292aa&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=148489417&repo=autoland
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/content/tests/browser/browser_autoplay_policy_play_twice.js:1:83 | Missing semicolon. (semi)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/content/tests/browser/browser_autoplay_policy_play_twice.js:40:2 | Unnecessary semicolon. (no-extra-semi)
Flags: needinfo?(alwu)
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0298b2f99059
part1 : bless media if media has started playing before. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/72b56f8f0d1a
part2 : add test. r=jwwang
https://hg.mozilla.org/mozilla-central/rev/0298b2f99059
https://hg.mozilla.org/mozilla-central/rev/72b56f8f0d1a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Blocks: 1424113
You need to log in before you can comment on or make changes to this bug.