-
Notifications
You must be signed in to change notification settings - Fork 382
Migrate Jetpack shortcode handling out of AMP plugin and into Jetpack #3309
Description
The AMP plugin includes shortcodes for several various embed handlers, including:
tweetvimeosoundcloudyoutubedailymotioninstagram
For example, the AMP plugin includes a vimeo shortcode as part of its AMP_Vimeo_Embed_Handler for historical reasons, but over time more and more Jetpack code has been moved from the AMP plugin to… Jetpack. See #1021. This code needs to be extracted from the AMP plugin to move over to Jetpack. All such shortcode handling needs to be moved to Jetpack since the shortcodes are only being registered for AMP responses, leaving non-AMP pages without any shortcode defined.
Hi there!
I am trying to convert my website's theme to be fully AMP compatible.
In the blog posts from my website, we use the Jetpack Vimeo shortcode. As the documentation states, we can use the shortcode by providing the URL or the video ID. We have used the shortcode exclusively using the [vimeo 123456] format, and it has worked as intended so far.
While testing the AMP version of my site, I noticed that the Vimeo videos were not showing. Further debugging took me to the AMP plugin, to the file includes/embeds/class-amp-vimeo-embed.php, to the function shortcode in line 71.
public function shortcode( $attr ) {
$video_id = false;
if ( isset( $attr['id'] ) ) {
$video_id = $attr['id'];
} elseif ( isset( $attr['url'] ) ) {
$video_id = $this->get_video_id_from_url( $attr['url'] );
} elseif ( isset( $attr[0] ) ) {
$video_id = $this->get_video_id_from_url( $attr[0] );
} elseif ( function_exists( 'shortcode_new_to_old_params' ) ) {
$video_id = shortcode_new_to_old_params( $attr );
}
if ( empty( $video_id ) ) {
return '';
}
return $this->render(
[
'video_id' => $video_id,
]
);
}The issue here is that the way the Jetpack documentation says it's possible to use the shortcode there is no need to use an id or url attribute, so the first two if statements do not apply to my case. Now, the third statement applies because I have submitted the Vimeo ID to the shortcode, and therefore $attr[0] is set. However, the statement assumes that I have passed a URL, as it calls get_video_id_from_url, where as I have passed an ID.
I think this shortcode function needs to parse the $attr variable first, similar to how Jetpack does it:
$attr = array_map(
'intval',
shortcode_atts(
array(
'id' => 0,
'width' => 0,
'height' => 0,
'autoplay' => 0,
'loop' => 0,
),
$atts
)
);
if ( isset( $atts[0] ) ) {
$attr['id'] = jetpack_shortcode_get_vimeo_id( $atts );
}
if ( ! $attr['id'] ) {
return '<!-- vimeo error: not a vimeo video -->';
}My other option is to do a search and replace on all my posts, but I think making the function be compatible with Jetpack's documentation would be easier.
Thanks!