diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index 284c1db04e7..9530665a0f2 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -368,38 +368,71 @@ private function adjust_and_replace_node( $node ) { */ private function maybe_add_lightbox_attributes( $attributes, $node ) { $parent_node = $node->parentNode; - if ( ! ( $parent_node instanceof DOMElement ) || 'figure' !== $parent_node->tagName ) { + if ( ! ( $parent_node instanceof DOMElement ) || ! ( $parent_node->parentNode instanceof DOMElement ) ) { return $attributes; } - // Account for blocks that include alignment. - // In that case, the structure changes from figure.wp-block-image > img + $is_file_url = preg_match( '/\.\w+$/', wp_parse_url( $parent_node->getAttribute( 'href' ), PHP_URL_PATH ) ); + $is_node_wrapped_in_media_file_link = ( + 'a' === $parent_node->tagName + && + ( 'figure' === $parent_node->tagName || 'figure' === $parent_node->parentNode->tagName ) + && + $is_file_url // This should be a link to the media file, not the attachment page. + ); + + if ( 'figure' !== $parent_node->tagName && ! $is_node_wrapped_in_media_file_link ) { + return $attributes; + } + + // Account for blocks that include alignment or images that are wrapped in . + // With alignment, the structure changes from figure.wp-block-image > img // to div.wp-block-image > figure > img and the amp-lightbox attribute // can be found on the wrapping div instead of the figure element. $grand_parent = $parent_node->parentNode; - if ( $grand_parent instanceof DOMElement ) { - $classes = preg_split( '/\s+/', $grand_parent->getAttribute( 'class' ) ); - if ( in_array( 'wp-block-image', $classes, true ) ) { - $parent_node = $grand_parent; - } + if ( $this->does_node_have_block_class( $grand_parent ) ) { + $parent_node = $grand_parent; + } elseif ( isset( $grand_parent->parentNode ) && $this->does_node_have_block_class( $grand_parent->parentNode ) ) { + $parent_node = $grand_parent->parentNode; } $parent_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $parent_node ); if ( isset( $parent_attributes['data-amp-lightbox'] ) && true === filter_var( $parent_attributes['data-amp-lightbox'], FILTER_VALIDATE_BOOLEAN ) ) { $attributes['data-amp-lightbox'] = ''; - $attributes['on'] = 'tap:' . self::AMP_IMAGE_LIGHTBOX_ID; - $attributes['role'] = 'button'; - $attributes['tabindex'] = 0; - - $this->maybe_add_amp_image_lightbox_node(); + $attributes['lightbox'] = ''; + + /* + * Removes the if the image is wrapped in one, as it can prevent the lightbox from working. + * But this only removes the if it links to the media file, not the attachment page. + */ + if ( $is_node_wrapped_in_media_file_link ) { + $node->parentNode->parentNode->replaceChild( $node, $node->parentNode ); + } } return $attributes; } /** - * Determines is a URL is considered a GIF URL + * Gets whether a node has the class 'wp-block-image', meaning it is a wrapper for an Image block. + * + * @param DOMElement $node A node to evaluate. + * @return bool Whether the node has the class 'wp-block-image'. + */ + private function does_node_have_block_class( $node ) { + if ( $node instanceof DOMElement ) { + $classes = preg_split( '/\s+/', $node->getAttribute( 'class' ) ); + if ( in_array( 'wp-block-image', $classes, true ) ) { + return true; + } + } + + return false; + } + + /** + * Determines if a URL is considered a GIF URL * * @since 0.2 * diff --git a/tests/php/test-amp-img-sanitizer.php b/tests/php/test-amp-img-sanitizer.php index 73b4fee4b00..627d51d8405 100644 --- a/tests/php/test-amp-img-sanitizer.php +++ b/tests/php/test-amp-img-sanitizer.php @@ -216,7 +216,7 @@ public function get_data() { 'image_with_custom_lightbox_attrs' => [ '
', - '
', + '
', ], 'wide_image' => [ @@ -336,12 +336,17 @@ public function get_data() { 'image_block_with_lightbox' => [ '
', - '
', + '
', + ], + + 'image_block_link_attach_page_lightbox' => [ + '
', + '
', ], 'aligned_image_block_with_lightbox' => [ '
', - '
', + '
', ], 'test_with_dev_mode' => [ @@ -438,4 +443,105 @@ public function test_no_gif_image_scripts() { ); $this->assertEquals( $expected, $scripts ); } + + /** + * Test an Image block wrapped in an , that links to the media file, with 'lightbox' selected. + * + * This should have the stripped, as it interferes with the lightbox. + */ + public function test_image_block_link_to_media_file_with_lightbox() { + $source = sprintf( '
', wp_get_attachment_image_url( $this->get_new_attachment_id() ) ); + $expected = '
'; + + $dom = AMP_DOM_Utils::get_dom_from_content( $source ); + $sanitizer = new AMP_Img_Sanitizer( $dom ); + $sanitizer->sanitize(); + + $sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom ); + $sanitizer->sanitize(); + $content = AMP_DOM_Utils::get_content_from_dom( $dom ); + $this->assertEquals( $expected, $content ); + } + + /** + * Test an Image block wrapped in an , that has right-alignment, links to the media file, and has 'lightbox' selected. + * + * This should have the stripped, as it interferes with the lightbox. + */ + public function test_image_block_link_to_media_file_and_alignment_with_lightbox() { + $source = sprintf( '
', wp_get_attachment_image_url( $this->get_new_attachment_id() ) ); + $expected = '
'; + + $dom = AMP_DOM_Utils::get_dom_from_content( $source ); + $sanitizer = new AMP_Img_Sanitizer( $dom ); + $sanitizer->sanitize(); + + $sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom ); + $sanitizer->sanitize(); + $content = AMP_DOM_Utils::get_content_from_dom( $dom ); + $this->assertEquals( $expected, $content ); + } + + /** + * Gets test data for test_does_node_have_block_class(), using a
element. + * + * @see AMP_Img_Sanitizer_Test::test_does_node_have_block_class() + * @return array Test data for function. + */ + public function get_data_for_node_block_class_test() { + return [ + 'has_no_class' => [ + '
', + false, + ], + 'has_wrong_class' => [ + '
', + false, + ], + 'only_has_part_of_class' => [ + '
', + false, + ], + 'has_correct_class' => [ + '
', + true, + ], + ]; + } + + /** + * Test does_node_have_block_class. + * + * @dataProvider get_data_for_node_block_class_test + * @covers \AMP_Img_Sanitizer::does_node_have_block_class() + * + * @param string $source The source markup to test. + * @param string $expected The expected return of the tested function, using the source markup. + * @throws ReflectionException If invoking the private method fails. + */ + public function test_does_node_have_block_class( $source, $expected ) { + $dom = AMP_DOM_Utils::get_dom_from_content( $source ); + $sanitizer = new AMP_Img_Sanitizer( $dom ); + $figures = $dom->getElementsByTagName( 'figure' ); + $method = new ReflectionMethod( $sanitizer, 'does_node_have_block_class' ); + + $method->setAccessible( true ); + $this->assertEquals( $expected, $method->invoke( $sanitizer, $figures->item( 0 ) ) ); + } + + /** + * Creates a new image attachment, and gets the ID. + * + * @return int|WP_Error The new attachment ID, or WP_Error. + */ + public function get_new_attachment_id() { + return $this->factory()->attachment->create_object( + 'example-image.jpeg', + $this->factory()->post->create(), + [ + 'post_mime_type' => 'image/jpeg', + 'post_type' => 'attachment', + ] + ); + } }