MP4 Video shortcode creates invalid AMP page
erralb opened this issue · 3 comments
As a user, I expect video shortcodes to work inside my WordPress posts when they are rendered as AMP.
- AC1: The sanitizer should convert the
http
request tohttps
as described below.
Hi,
You can see this problem directly within the AMP Google search console :
https://search.google.com/test/amp?skip_amp_follow=true&id=RH1_TwbRhxkrC2y1Hgap-Q
The post content generating the error is :
[video width="740" height="428" mp4="http://ads-up.fr/app/uploads/2016/06/BingAdsEditor-Mac-Multicomptes.mp4" loop="true" autoplay="true" preload="auto"][/video]
The AMP generated code is :
[video width="740" height="428" mp4="http://ads-up.fr/app/uploads/2016/06/BingAdsEditor-Mac-Multicomptes.mp4" loop="true" autoplay="true" preload="auto"][/video]
Yes, this is a problem I've come across as well. The quick fix on your end is to just modify the URL from http:
to https:
.
However, this should be getting done automatically, such in AMP_Video_Sanitizer
by:
I think the problem is just that the maybe_enforce_https_src
method is not applying to the src
attribute on source
elements, which is what the video
shortcode is (correctly) outputting for you:
<amp-video class="wp-video-shortcode amp-wp-enforced-sizes" width="740" height="428" loop="" autoplay="" controls="" sizes="(min-width: 600px) 600px, 100vw">
<source type="video/mp4" src="http://ads-up.fr/app/uploads/2016/06/BingAdsEditor-Mac-Multicomptes.mp4?_=1"/>
</amp-video>
See also #969.
Hi @ierpe, We plan to work on this as part of our v1.0 release. I've added a user story and acceptance criteria for one of the XWP engineers to work on this. Thanks for submitting this issue!
Moving To "Ready For Merging"
If it's alright, I'm moving this to "Ready For Merging." If you think this could use functional testing, feel free to move it back.