-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🐛 [Story Preview] Enable amp-video to play in preview mode #38149
Conversation
This pull request introduces 2 alerts when merging 2b8f1c2 into d365219 - view on LGTM.com new alerts:
|
Hey @gmajoulet, @newmuis! These files were changed:
|
I'm testing the case where the video's cached source fails to load during preview mode and the origin source subsequently loads successfully in visible mode. My test story has a video with a single source: The one issue that remains here is that the |
I've updated the test amp-video's sources as follows:
However, I'm experiencing a testing issue where the video actually plays successfully in preview mode via the |
…PlayUnplayedVideos_()
I managed to successfully test using my demo site by blocking the requests for the cached video sources using DevTools. As seen in the video below, the following occurs:
previewPlayVideo.mov |
…a-menu-images-validator-spec * 'main' of github.com:ampproject/amphtml: (90 commits) 🔥 [Story mediapool] Fix videos from mediapool with `noaudio` don't have audio when reused. (#38216) Hide progress bar on the control group of auto advance experiment (#38215) ✨ Add Bento Autocomplete Component (#37837) 🌐 [Story subscription] Subscription localization async (#38204) Dable: add new optional parameter "channel" (#38199) ✨ [AMP Story Paywall] Enable developers to specify a custom subscriptions page index (#38175) SwG Release 0.1.22.217 (#38187) amp-script: implements new size limits for sandboxed scripts (#38185) 🖍 Hide the system layer and progress bar in preview mode (#38163) added minItems (#38177) Prevent expandTemplate from ReDOSing (#38178) Change amp-story-subscriptions attribute name to reflect its flexibility (#38176) 🐛 [Story Preview] Enable amp-video to play in preview mode (#38149) Added the possibility to get page count to story messaging api (#38170) SwG Release 0.1.22.216 (#38168) Allow @newmuis to update OWNERS files (#38169) ✨ Add Richaudience to RTC callout vendors (#38160) 🚀 SunMedia: Update amp-ad (#38128) Remove option to deploy PR artifacts to a static website (#38152) added some vars and requests in gfksensic.json (#37722) ...
PREVIEW
visibility state #37129Although this PR enabled stories to play in preview mode, video elements still fail to play. This failure occurs due to the
amp-video
layout callback being unable to resolve in any non-visible
visibility state, which ultimately blocksamp-story-page
from being able to register and play the video.Instead of indefinitely blocking registration, this PR registers media elements at the beginning of their layout callbacks, enabling videos in preview mode to play as long as they have cached sources.