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
Disable option to activate autoplay with YT. #3794
Conversation
While autoplay is landing in mobile browsers (for now it is still blocked in Safari and Chrome), we deactivate this for now to ensure the experience if good when it becomes available. Autoplay support will be added back later this year with support for features like - no autoplay off screen - mute by default - only autoplay a single video per page
I put data-param-autoplay as an example in the doc and examples. |
@muxin Good point! Done. |
expect(iframe.src).to.contain('myParam=hello%20world'); | ||
// autoplay is temporarily black listed. | ||
expect(iframe.src).to.not.contain('autoplay=1'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm testing that it gets removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test is failing because of re-calculating the params - as I mentioned in a comment above.
if ('autoplay' in params) { | ||
delete params['autoplay']; | ||
user.warn('Autoplay is currently not support with amp-youtube.'); | ||
} | ||
src = addParamsToUrl(src, getDataParamsFromAttributes(this.element)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually doesn't use the params const above and it recalculates the params. Please fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 🐑
Also documents the change.
LGTM |
While autoplay is landing in mobile browsers (for now it is still blocked in Safari and Chrome), we deactivate this for now to ensure the experience if good when it becomes available. Autoplay support will be added back later this year with support for features like