Improve handling of unlisted Vimeo videos (#2986)

ampproject · Aug 12, 2019 · ea9e6fb · ea9e6fb
1 parent 2987b5f
commit ea9e6fb
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 6 deletions.
diff --git a/includes/embeds/class-amp-vimeo-embed.php b/includes/embeds/class-amp-vimeo-embed.php
@@ -156,16 +156,15 @@ public function render( $args ) {
 	 * Determine the video ID from the URL.
 	 *
 	 * @param string $url URL.
-	 * @return integer Video ID.
+	 * @return int Video ID.
 	 */
 	private function get_video_id_from_url( $url ) {
-		$parsed_url = wp_parse_url( $url );
-		parse_str( $parsed_url['path'], $path );
+		$path = wp_parse_url( $url, PHP_URL_PATH );
 
+		// @todo This will not get the private key for unlisted videos (which look like https://vimeo.com/123456789/abcdef0123), but amp-vimeo doesn't support them currently anyway.
 		$video_id = '';
-		if ( $path ) {
-			$tok      = explode( '/', $parsed_url['path'] );
-			$video_id = end( $tok );
+		if ( $path && preg_match( ':^/(\d+):', $path, $matches ) ) {
+			$video_id = $matches[1];
 		}
 
 		return $video_id;

diff --git a/tests/php/test-amp-vimeo-embed.php b/tests/php/test-amp-vimeo-embed.php
@@ -1,6 +1,22 @@
 <?php
+/**
+ * Class AMP_Vimeo_Embed_Test
+ *
+ * @package AMP
+ */
 
+/**
+ * Class AMP_Vimeo_Embed_Test
+ *
+ * @covers AMP_Vimeo_Embed_Handler
+ */
 class AMP_Vimeo_Embed_Test extends WP_UnitTestCase {
+
+	/**
+	 * Get conversion data.
+	 *
+	 * @return array
+	 */
 	public function get_conversion_data() {
 		return [
 			'no_embed'                      => [
@@ -13,6 +29,11 @@ public function get_conversion_data() {
 				'<p><amp-vimeo data-videoid="172355597" layout="responsive" width="600" height="338"></amp-vimeo></p>' . PHP_EOL,
 			],
 
+			'url_unlisted'                  => [
+				'https://vimeo.com/172355597/abcdef0123' . PHP_EOL,
+				'<p><amp-vimeo data-videoid="172355597" layout="responsive" width="600" height="338"></amp-vimeo></p>' . PHP_EOL,
+			],
+
 			'shortcode_unnamed_attr_as_url' => [
 				'[vimeo https://vimeo.com/172355597]' . PHP_EOL,
 				'<amp-vimeo data-videoid="172355597" layout="responsive" width="600" height="338"></amp-vimeo>' . PHP_EOL,
@@ -32,7 +53,12 @@ public function get_conversion_data() {
 	}
 
 	/**
+	 * Test conversion.
+	 *
 	 * @dataProvider get_conversion_data
+	 *
+	 * @param string $source   Source.
+	 * @param string $expected Expected.
 	 */
 	public function test__conversion( $source, $expected ) {
 		$embed = new AMP_Vimeo_Embed_Handler();
@@ -42,6 +68,11 @@ public function test__conversion( $source, $expected ) {
 		$this->assertEquals( $expected, $filtered_content );
 	}
 
+	/**
+	 * Get scripts data.
+	 *
+	 * @return array Scripts data.
+	 */
 	public function get_scripts_data() {
 		return [
 			'not_converted' => [
@@ -56,7 +87,12 @@ public function get_scripts_data() {
 	}
 
 	/**
+	 * Test get_scripts().
+	 *
 	 * @dataProvider get_scripts_data
+	 *
+	 * @param string $source   Source.
+	 * @param string $expected Expected.
 	 */
 	public function test__get_scripts( $source, $expected ) {
 		$embed = new AMP_Vimeo_Embed_Handler();