Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lightbox support to Core Image blocks with links #3460

Merged
merged 9 commits into from
Oct 21, 2019
Merged

Add lightbox support to Core Image blocks with links #3460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c4dd0ec
Allow images that are wrapped in <a> to have a lightbox
kienstra Oct 7, 2019
237cde9
Use amp-lightbox-gallery for Core Image block
kienstra Oct 15, 2019
9ee7d2e
Add a comment that code can be removed if an issue is implemented
kienstra Oct 15, 2019
9c7fd7b
Only remove the <a> if it goes to the media file, not the attachment …
kienstra Oct 17, 2019
9a08c8a
Allow lightboxes in Image blocks with links and alignment
kienstra Oct 17, 2019
88b8efc
Add a parameter to the invoke() method in a test
kienstra Oct 17, 2019
bd1906b
Remove the link to the amphtml issue, as it's closed
kienstra Oct 17, 2019
d243326
Access a DOMNodeList item with ->item() instead of [0]
kienstra Oct 17, 2019
783a8d8
Use Weston's method of finding if the link is to a file
kienstra Oct 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 47 additions & 14 deletions includes/sanitizers/class-amp-img-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) );
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
$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 <a>.
// 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;
}
Copy link
Contributor Author

@kienstra kienstra Oct 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic above is mainly copied into does_node_have_block_class()

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 <a> if the image is wrapped in one, as it can prevent the lightbox from working.
* But this only removes the <a> 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 );
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If amphtml later supports wrapping <amp-img lightbox> in <a>, this can be removed.

}

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
*
Expand Down
112 changes: 109 additions & 3 deletions tests/php/test-amp-img-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public function get_data() {

'image_with_custom_lightbox_attrs' => [
'<figure data-amp-lightbox="true"><img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" /></figure>',
'<figure data-amp-lightbox="true"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" on="tap:amp-image-lightbox" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></figure><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
'<figure data-amp-lightbox="true"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" lightbox="" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></figure>',
],

'wide_image' => [
Expand Down Expand Up @@ -336,12 +336,17 @@ public function get_data() {

'image_block_with_lightbox' => [
'<figure class="wp-block-image" data-amp-lightbox="true"><img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" /></figure>',
'<figure class="wp-block-image" data-amp-lightbox="true"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" on="tap:amp-image-lightbox" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></figure><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
'<figure class="wp-block-image" data-amp-lightbox="true"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" lightbox="" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></figure>',
],

'image_block_link_attach_page_lightbox' => [
'<figure class="wp-block-image" data-amp-lightbox="true"><a href="https://example.com/example-image"><img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" /></a></figure>',
'<figure class="wp-block-image" data-amp-lightbox="true"><a href="https://example.com/example-image"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></a></figure>',
],

'aligned_image_block_with_lightbox' => [
'<div data-amp-lightbox="true" class="wp-block-image"><figure class="alignleft is-resized"><img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" /></figure></div>',
'<div data-amp-lightbox="true" class="wp-block-image"><figure class="alignleft is-resized"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" on="tap:amp-image-lightbox" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></figure></div><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
'<div data-amp-lightbox="true" class="wp-block-image"><figure class="alignleft is-resized"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" lightbox="" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></figure></div>',
],

'test_with_dev_mode' => [
Expand Down Expand Up @@ -438,4 +443,105 @@ public function test_no_gif_image_scripts() {
);
$this->assertEquals( $expected, $scripts );
}

/**
* Test an Image block wrapped in an <a>, that links to the media file, with 'lightbox' selected.
*
* This should have the <a> stripped, as it interferes with the lightbox.
*/
public function test_image_block_link_to_media_file_with_lightbox() {
$source = sprintf( '<figure class="wp-block-image" data-amp-lightbox="true"><a href="%s"><img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" /></a></figure>', wp_get_attachment_image_url( $this->get_new_attachment_id() ) );
$expected = '<figure class="wp-block-image" data-amp-lightbox="true"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" lightbox="" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></figure>';

$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 <a>, that has right-alignment, links to the media file, and has 'lightbox' selected.
*
* This should have the <a> stripped, as it interferes with the lightbox.
*/
public function test_image_block_link_to_media_file_and_alignment_with_lightbox() {
$source = sprintf( '<div data-amp-lightbox="true" class="wp-block-image"><figure class="alignright size-large"><a href="%s"><img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" /></a></figure></div>', wp_get_attachment_image_url( $this->get_new_attachment_id() ) );
$expected = '<div data-amp-lightbox="true" class="wp-block-image"><figure class="alignright size-large"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" lightbox="" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></figure></div>';

$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 <figure> 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' => [
'<figure></figure>',
false,
],
'has_wrong_class' => [
'<figure class="completely-wrong-class"></figure>',
false,
],
'only_has_part_of_class' => [
'<figure class="wp-block"></figure>',
false,
],
'has_correct_class' => [
'<figure class="wp-block-image"></figure>',
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',
]
);
}
}