From 709e87ab525a66b223b53140af02c4228f0beb71 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Fri, 25 May 2018 18:01:02 -0500 Subject: [PATCH 1/3] Display schema.org image data for 'attachment' post type. As reported on a support topic, on Native AMP, on 'attachment' pages, there's no schema.org data for the image. So add a conditional to check for this post type. It's possible that the attachment is a video, like a .mov file. In that case, it won't return an image. wp_get_attachment_image_src() will return false. @see https://wordpress.org/support/topic/structured-data-error-for-attachment-pages-media/ --- includes/amp-helper-functions.php | 2 ++ tests/test-amp-helper-functions.php | 45 +++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 27d27696058..d1d315b7a67 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -524,6 +524,8 @@ function amp_get_post_image_metadata( $post = null ) { if ( has_post_thumbnail( $post->ID ) ) { $post_image_id = get_post_thumbnail_id( $post->ID ); + } elseif ( 'attachment' === $post->post_type ) { + $post_image_id = $post->ID; } else { $attached_image_ids = get_posts( array( diff --git a/tests/test-amp-helper-functions.php b/tests/test-amp-helper-functions.php index b29f3eefaa7..263006f64ce 100644 --- a/tests/test-amp-helper-functions.php +++ b/tests/test-amp-helper-functions.php @@ -372,6 +372,51 @@ public function test_amp_get_post_image_metadata() { ) ); $metadata = amp_get_post_image_metadata( $post_id ); $this->assertStringEndsWith( 'test-image.jpg', $metadata['url'] ); + + // Test an 'attachment' post type. + $attachment_src = 'example/attachment.jpeg'; + $attachment_height = 45; + $attachment_width = 600; + $attachment_id = $this->factory()->attachment->create_object( + $attachment_src, + 0, + array( + 'post_mime_type' => 'image/jpeg', + ) + ); + $expected_attachment_img = wp_get_attachment_image_src( $attachment_id, 'full', false ); + + update_post_meta( + $attachment_id, + '_wp_attachment_metadata', + array( + 'height' => $attachment_height, + 'width' => $attachment_width, + ) + ); + $this->go_to( get_permalink( $attachment_id ) ); + + $this->assertEquals( + array( + '@type' => 'ImageObject', + 'height' => $attachment_height, + 'url' => $expected_attachment_img[0], + 'width' => $attachment_width, + ), + amp_get_post_image_metadata( $attachment_id ) + ); + + // Test a video as an 'attachment' post type, which shouldn't have a schema.org image. + $attachment_src = 'example/test-video.mpeg'; + $attachment_id = $this->factory()->attachment->create_object( + $attachment_src, + 0, + array( + 'post_mime_type' => 'video/mpeg', + ) + ); + $this->go_to( get_permalink( $attachment_id ) ); + $this->assertNull( amp_get_post_image_metadata( $attachment_id ) ); } /** From 3dde8f7e4d5c630b3b1336dd54fa243a8e02c853 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Fri, 25 May 2018 18:08:16 -0500 Subject: [PATCH 2/3] Align = signs vertically. There was a failed Travis build. So align the = signs with line 387. --- tests/test-amp-helper-functions.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test-amp-helper-functions.php b/tests/test-amp-helper-functions.php index 263006f64ce..03cf8c4318d 100644 --- a/tests/test-amp-helper-functions.php +++ b/tests/test-amp-helper-functions.php @@ -374,10 +374,10 @@ public function test_amp_get_post_image_metadata() { $this->assertStringEndsWith( 'test-image.jpg', $metadata['url'] ); // Test an 'attachment' post type. - $attachment_src = 'example/attachment.jpeg'; - $attachment_height = 45; - $attachment_width = 600; - $attachment_id = $this->factory()->attachment->create_object( + $attachment_src = 'example/attachment.jpeg'; + $attachment_height = 45; + $attachment_width = 600; + $attachment_id = $this->factory()->attachment->create_object( $attachment_src, 0, array( From 15ea3d94c8336c1d614384c6d817ea3dec8a17a4 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Fri, 25 May 2018 19:30:46 -0500 Subject: [PATCH 3/3] Check that the attachment is an image. Props @westonruter for the conditional check. This ensures that the attachment isn't another type, like a video. In that case, wp_get_attachment_image_src() won't return image data. --- includes/amp-helper-functions.php | 2 +- tests/test-amp-helper-functions.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index d1d315b7a67..b357fcfc3fd 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -524,7 +524,7 @@ function amp_get_post_image_metadata( $post = null ) { if ( has_post_thumbnail( $post->ID ) ) { $post_image_id = get_post_thumbnail_id( $post->ID ); - } elseif ( 'attachment' === $post->post_type ) { + } elseif ( ( 'attachment' === $post->post_type ) && wp_attachment_is( 'image', $post ) ) { $post_image_id = $post->ID; } else { $attached_image_ids = get_posts( diff --git a/tests/test-amp-helper-functions.php b/tests/test-amp-helper-functions.php index 03cf8c4318d..327e021ff6c 100644 --- a/tests/test-amp-helper-functions.php +++ b/tests/test-amp-helper-functions.php @@ -416,7 +416,7 @@ public function test_amp_get_post_image_metadata() { ) ); $this->go_to( get_permalink( $attachment_id ) ); - $this->assertNull( amp_get_post_image_metadata( $attachment_id ) ); + $this->assertFalse( amp_get_post_image_metadata( $attachment_id ) ); } /**