Skip to content

Migrate Jetpack shortcode handling out of AMP plugin and into Jetpack #3309

@chvillanuevap

Description

@chvillanuevap

The AMP plugin includes shortcodes for several various embed handlers, including:

  • tweet
  • vimeo
  • soundcloud
  • youtube
  • dailymotion
  • instagram

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!

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions