Amphtml: amp-story MediaPool breaks media-based auto-advance

[newmuis]

amp-story has a feature where it can auto-advance to the next page based on the completion of the playback of a media element on the current page. This media element is specified by its ID, in the auto-advance-after attribute of the current page. For example:

<amp-story-page id="page1" auto-advance-after="my-vid">
  <amp-story-grid-layer template="fill">
    <amp-video id="my-vid" src="cat.mp4" muted autoplay playsinline></amp-video>
  </amp-story-grid-layer>
</amp-story-page>

However, MediaPool swaps out all media elements when the page becomes visible, so the video actually specified when the document is published (my-vid, in the example above) is not the video that is in the DOM when the page becomes active.

We will need to somehow plumb this through.

[sanjsanj]

@newmuis A partial fix to this would be to add 'auto-advance-after' to:

https://github.com/ampproject/amphtml/blob/71b4031aee2ff9492c98f51f598b6031dd3ac83e/extensions/amp-story/0.1/media-pool.js#L66-L75

It actually works great but on my test page with 2 amp-videos I get this console error:

Uncaught (in promise) DOMException: The play() request was interrupted by a call to pause(). https://goo.gl/LdLk22

[sanjsanj]

edit: ninja edit of code block above

[sanjsanj]

You can recreate the above error by:

1 - Loading up amphtml/examples/amp-story/progress-bar.html
2 - Navigate forward to page 4 (the 2nd video)
3 - Navigate back to page 2 (a regular auto-advance page)
4 - Let the amp-story auto-advance forward to page 4

[newmuis]

Actually, I'm having trouble reproducing this. My initial report is mistaken because the media element that is swapped is the video tag added by amp-video, and not amp-video itself. As such, the ID my-vid stays in the document at all times, and so adding the event listener to listen to ended and timeupdate events works just fine.

I don't think we should need to add auto-advance-after to the PROTECTED_ATTRIBUTES, as PROTECTED_ATTRIBUTES is only for attributes on the swapped media element, whereas the auto-advance-after attribute appears on the amp-story-page.

@sanjsanj, I'll try updating amphtml/examples/amp-story/progress-bar.html with an example.

[sanjsanj]

@newmuis Ok thanks for the update! If there's anything I can pick up to help please shout. I don't know if you've seen it but I have a PR in to update the progress-bar.html as well if you can have a look please, I hadn't set it up as well as I could have and also added an amp-audio, thanks! #12926

[newmuis]

Sorry, I totally missed #12926! Approved and merged it now; I'll use that to verify whether this issue can be marked as fixed.

[sanjsanj]

Nice one!

[newmuis]

Yep, this example seems to work fine. Thanks for the work on this @sanjsanj!