1461877 - Youtube player can't play if blocked autoplay ad times out

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P2)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

As per bug 1453843 comment 23, if you disable autoplay and let a YouTube ad timeout, the subsequent content video does not play; YouTube's player thinks it's playing, but the video isn't playing.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

This is caused by us enqueuing a task to dispatch "playing" here https://searchfox.org/mozilla-central/rev/00dd116638001772fe354b081353b73f1cad405d/dom/html/HTMLMediaElement.cpp#4133 when we subsequently reject the play promise before the enqueued "playing" is actually dispatched, and when the "playing" event finally dispatches it confuses YouTube's player.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1461877 - Ensure we don't dispatch 'playing' when we're about to reject pending play promises.

Currently we can end up dispatching a 'playing' event right before we reject
play() promises, and this confuses YouTube's controls, and it doesn't make
sense to dispatch a 'playing' event when we're not playing anyway.

This is because the logic to delay resolving the play() promise until after
we've reached loadedmetadata doesn't prevent the 'playing' event from being
dispatched. We shouldn't dispatch 'playing' until we resolve the play()
promise(s).

Review commit: https://reviewboard.mozilla.org/r/244554/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/244554/

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1461877 - Test that we don't dispatch 'playing' event for blocked playbacks.

Review commit: https://reviewboard.mozilla.org/r/244830/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/244830/

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8976405 [details]
Bug 1461877 - Ensure we don't dispatch 'playing' when we're about to reject pending play promises.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/244554/diff/1-2/

Comment 5

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 8976742 [details]
Bug 1461877 - Test that we don't dispatch 'playing' event for blocked playbacks.

https://reviewboard.mozilla.org/r/244830/#review251086

Comment 6

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 8976405 [details]
Bug 1461877 - Ensure we don't dispatch 'playing' when we're about to reject pending play promises.

https://reviewboard.mozilla.org/r/244554/#review251134

LGTM. To confirm, we want fire play[0] when blocking autoplay, but not the playing event? I.e. if we block we fire 'play' and 'paused' and we also reject the promise from play()?

[0]: https://searchfox.org/mozilla-central/rev/da499aac682d0bbda5829327b60a865cbc491611/dom/html/HTMLMediaElement.cpp#4097

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Bryce Van Dyk (:bryce) from comment #6)
> Comment on attachment 8976405 [details]
> Bug 1461877 - Ensure we don't dispatch 'playing' when we're about to reject
> pending play promises.
> 
> https://reviewboard.mozilla.org/r/244554/#review251134
> 
> LGTM. To confirm, we want fire play[0] when blocking autoplay, but not the
> playing event?

Yes, we fire "play" when the media element switches to "potentially playing" state (when the paused attribute switches to false). We subsequently find out we can't play later, so we reject the play promise, and fire the "pause" event.

We fire "playing" when we've actually succeeded in playing, in advancing HTMLMediaElement.currentTime.

> I.e. if we block we fire 'play' and 'paused' and we also
> reject the promise from play()?

Yes.

I'm going to investigate a little more whether we would be better to not fire "play" in the case where we need more information before we can reject the play promise. I'm a little worried that too many sites will rely on checking HTMLMediaElement.paused synchronously right after calling play(), so I need time to investigate it more carefully. But I think it makes sense to land the patches here now, to fix YouTube.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e1eb21be77ba
Test that we don't dispatch 'playing' event for blocked playbacks. r=bryce
https://hg.mozilla.org/integration/autoland/rev/25513d2e78e9
Ensure we don't dispatch 'playing' when we're about to reject pending play promises. r=bryce

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

I think the behaviour of firing "play" when we later reject the play() promise due to loading to metadata discovering the media is audible is OK. It's in line with the spec https://html.spec.whatwg.org/multipage/media.html#dom-media-play where this is similar to the the case where play() is called while readyState==HAVE_NOTHING and we must wait until we've loaded something before we can play.

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/e1eb21be77ba
https://hg.mozilla.org/mozilla-central/rev/25513d2e78e9