From 0df34eafe18a35693e1d1484705ad845d466806f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 24 Aug 2021 13:02:04 -0700 Subject: [PATCH 1/6] Prevent amp-tiktok/blockquote from being double-converted --- .../embeds/class-amp-tiktok-embed-handler.php | 16 +---- .../test-class-amp-tiktok-embed-handler.php | 68 ++++++++++++++++--- 2 files changed, 61 insertions(+), 23 deletions(-) diff --git a/includes/embeds/class-amp-tiktok-embed-handler.php b/includes/embeds/class-amp-tiktok-embed-handler.php index 9cfbb3fb463..c5a4b1ca838 100644 --- a/includes/embeds/class-amp-tiktok-embed-handler.php +++ b/includes/embeds/class-amp-tiktok-embed-handler.php @@ -34,27 +34,13 @@ public function unregister_embed() { * @param Document $dom DOM. */ public function sanitize_raw_embeds( Document $dom ) { - $nodes = $dom->xpath->query( '//blockquote[ contains( @class, "tiktok-embed" ) ]' ); + $nodes = $dom->xpath->query( '//blockquote[ contains( @class, "tiktok-embed" ) and not( parent::amp-tiktok ) ]' ); foreach ( $nodes as $node ) { - if ( ! $this->is_raw_embed( $node ) ) { - continue; - } - $this->make_embed_amp_compatible( $node ); } } - /** - * Determine if the node has already been sanitized. - * - * @param DOMElement $node The DOMNode. - * @return bool Whether the node is a raw embed. - */ - protected function is_raw_embed( DOMElement $node ) { - return ! $node->firstChild || ( $node->firstChild && 'amp-embedly-card' !== $node->firstChild->nodeName ); - } - /** * Make TikTok embed AMP compatible. * diff --git a/tests/php/test-class-amp-tiktok-embed-handler.php b/tests/php/test-class-amp-tiktok-embed-handler.php index f052a76855b..0f431b69548 100644 --- a/tests/php/test-class-amp-tiktok-embed-handler.php +++ b/tests/php/test-class-amp-tiktok-embed-handler.php @@ -5,6 +5,7 @@ * @package AMP. */ +use AmpProject\AmpWP\Tests\Helpers\MarkupComparison; use AmpProject\AmpWP\Tests\Helpers\WithoutBlockPreRendering; use AmpProject\AmpWP\Tests\TestCase; @@ -13,6 +14,8 @@ */ class Test_AMP_TikTok_Embed_Handler extends TestCase { + use MarkupComparison; + use WithoutBlockPreRendering { setUp as public prevent_block_pre_render; } @@ -70,17 +73,61 @@ public function mock_http_request( $pre, $r, $url ) { */ public function get_conversion_data() { return [ - 'no_embed' => [ + 'no_embed' => [ '

Hello world.

', '

Hello world.

' . PHP_EOL, ], - 'url_simple' => [ + 'url_simple' => [ 'https://www.tiktok.com/@scout2015/video/6718335390845095173' . PHP_EOL, - '
@scout2015 ' . PHP_EOL . - '

Scramble up ur name & I’ll try to guess it😍❤️ #foryoupage #petsoftiktok #aesthetic

' . PHP_EOL . - '

♬ original sound – tiff

' . PHP_EOL . PHP_EOL, + ' + +
+ @scout2015 +

Scramble up ur name & I’ll try to guess it😍❤️ #foryoupage + #petsoftiktok + #aesthetic +

+

+ ♬ original sound – tiff +

+
+ + ', + ], + + 'amp-tiktok-embed-code' => [ + ' +
@countingprimes

You can now embed TikTok\'s in AMP

♬ original sound - countingprimes
+ ', + ' + +
+ @countingprimes +

You can now embed TikTok’s in AMP

+

♬ original sound – countingprimes

+
+ + ', + ], + + 'amp-tiktok-passthrough' => [ + ' + + +
+
+ @countingprimes +

You can now embed TikTok’s in AMP

+ ♬ original sound — countingprimes +
+
+ + + ', + + null, ], ]; } @@ -92,10 +139,15 @@ public function get_conversion_data() { * @param string $expected Expected. * @dataProvider get_conversion_data */ - public function test_conversion( $source, $expected ) { + public function test_conversion( $source, $expected = null ) { if ( version_compare( '5.4-alpha', get_bloginfo( 'version' ), '>' ) ) { $this->markTestSkipped( 'The TikTok embed is only available in 5.4-alpha (until 5.4 is stable)' ); } + if ( ! $expected ) { + $expected = $source; + } + + $expected = preg_replace( '//s', '', $expected ); $embed = new AMP_TikTok_Embed_Handler(); $embed->register_embed(); @@ -104,8 +156,8 @@ public function test_conversion( $source, $expected ) { $dom = AMP_DOM_Utils::get_dom_from_content( $filtered_content ); $embed->sanitize_raw_embeds( $dom ); - $content = AMP_DOM_Utils::get_content_from_dom( $dom ); + $actual = AMP_DOM_Utils::get_content_from_dom( $dom ); - $this->assertEquals( $expected, $content ); + $this->assertSimilarMarkup( $expected, $actual ); } } From fcc5af201e5972b09b6b88d09eea2d2d03cf6469 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 24 Aug 2021 13:18:19 -0700 Subject: [PATCH 2/6] Use amp-tiktok instead of amp-embedly-card --- .../embeds/class-amp-tiktok-embed-handler.php | 75 ++++++++----------- .../test-class-amp-tiktok-embed-handler.php | 35 ++++----- 2 files changed, 47 insertions(+), 63 deletions(-) diff --git a/includes/embeds/class-amp-tiktok-embed-handler.php b/includes/embeds/class-amp-tiktok-embed-handler.php index c5a4b1ca838..bee3c8febfd 100644 --- a/includes/embeds/class-amp-tiktok-embed-handler.php +++ b/includes/embeds/class-amp-tiktok-embed-handler.php @@ -5,7 +5,12 @@ * @package AMP */ +use AmpProject\Attribute; use AmpProject\Dom\Document; +use AmpProject\Dom\Element; +use AmpProject\Extension; +use AmpProject\Layout; +use AmpProject\Tag; /** * Class AMP_TikTok_Embed_Handler @@ -34,7 +39,7 @@ public function unregister_embed() { * @param Document $dom DOM. */ public function sanitize_raw_embeds( Document $dom ) { - $nodes = $dom->xpath->query( '//blockquote[ contains( @class, "tiktok-embed" ) and not( parent::amp-tiktok ) ]' ); + $nodes = $dom->xpath->query( '//blockquote[ @cite and @data-video-id and contains( @class, "tiktok-embed" ) and not( parent::amp-tiktok ) ]' ); foreach ( $nodes as $node ) { $this->make_embed_amp_compatible( $node ); @@ -44,71 +49,53 @@ public function sanitize_raw_embeds( Document $dom ) { /** * Make TikTok embed AMP compatible. * - * @param DOMElement $blockquote_node The
node to make AMP compatible. + * @param Element $blockquote The
node to make AMP compatible. */ - protected function make_embed_amp_compatible( DOMElement $blockquote_node ) { - $dom = $blockquote_node->ownerDocument; - $video_url = $blockquote_node->getAttribute( 'cite' ); + protected function make_embed_amp_compatible( Element $blockquote ) { + $dom = $blockquote->ownerDocument; + $video_url = $blockquote->getAttribute( Attribute::CITE ); - // If there is no video ID, stop here as its needed for the `data-url` parameter. + // If there is no video ID, stop here as its needed for the `data-src` parameter. if ( empty( $video_url ) ) { return; } - $this->remove_embed_script( $blockquote_node ); + $this->remove_embed_script( $blockquote ); - $amp_node = AMP_DOM_Utils::create_node( + $amp_tiktok = AMP_DOM_Utils::create_node( Document::fromNode( $dom ), - 'amp-embedly-card', + Extension::TIKTOK, [ - 'layout' => 'responsive', - 'height' => 700, - 'width' => 340, - 'data-card-controls' => 0, - 'data-url' => $video_url, + Attribute::LAYOUT => Layout::RESPONSIVE, + Attribute::HEIGHT => 575, + Attribute::WIDTH => 325, + Attribute::DATA_SRC => $video_url, ] ); - // Find existing
node to use as the placeholder. - foreach ( iterator_to_array( $blockquote_node->childNodes ) as $child ) { - if ( ! ( $child instanceof DOMElement ) ) { - continue; - } - - // Append the placeholder if it was found. - if ( 'section' === $child->nodeName ) { - /** - * Placeholder to append to the AMP component. - * - * @var DOMElement $placeholder_node - */ - $placeholder_node = $blockquote_node->removeChild( $child ); - $placeholder_node->setAttribute( 'placeholder', '' ); - $amp_node->appendChild( $placeholder_node ); - break; - } - } - - $blockquote_node->parentNode->replaceChild( $amp_node, $blockquote_node ); + $blockquote->parentNode->replaceChild( $amp_tiktok, $blockquote ); + $amp_tiktok->appendChild( $blockquote ); + $blockquote->setAttributeNode( $dom->createAttribute( Attribute::PLACEHOLDER ) ); + $blockquote->removeAttribute( Attribute::STYLE ); } /** * Remove the TikTok embed script if it exists. * - * @param DOMElement $node The DOMNode to make AMP compatible. + * @param Element $blockquote The blockquote element being made AMP-compatible. */ - protected function remove_embed_script( DOMElement $node ) { - $next_element_sibling = $node->nextSibling; - while ( $next_element_sibling && ! ( $next_element_sibling instanceof DOMElement ) ) { + protected function remove_embed_script( Element $blockquote ) { + $next_element_sibling = $blockquote->nextSibling; + while ( $next_element_sibling && ! ( $next_element_sibling instanceof Element ) ) { $next_element_sibling = $next_element_sibling->nextSibling; } $script_src = 'tiktok.com/embed.js'; // Handle case where script is wrapped in paragraph by wpautop. - if ( $next_element_sibling instanceof DOMElement && 'p' === $next_element_sibling->nodeName ) { + if ( $next_element_sibling instanceof Element && Tag::P === $next_element_sibling->nodeName ) { $children = $next_element_sibling->getElementsByTagName( '*' ); - if ( 1 === $children->length && 'script' === $children->item( 0 )->nodeName && false !== strpos( $children->item( 0 )->getAttribute( 'src' ), $script_src ) ) { + if ( 1 === $children->length && Tag::SCRIPT === $children->item( 0 )->nodeName && false !== strpos( $children->item( 0 )->getAttribute( Attribute::SRC ), $script_src ) ) { $next_element_sibling->parentNode->removeChild( $next_element_sibling ); return; } @@ -116,11 +103,11 @@ protected function remove_embed_script( DOMElement $node ) { // Handle case where script is immediately following. $is_embed_script = ( - $next_element_sibling instanceof DOMElement + $next_element_sibling instanceof Element && - 'script' === strtolower( $next_element_sibling->nodeName ) + Tag::SCRIPT === strtolower( $next_element_sibling->nodeName ) && - false !== strpos( $next_element_sibling->getAttribute( 'src' ), $script_src ) + false !== strpos( $next_element_sibling->getAttribute( Attribute::SRC ), $script_src ) ); if ( $is_embed_script ) { $next_element_sibling->parentNode->removeChild( $next_element_sibling ); diff --git a/tests/php/test-class-amp-tiktok-embed-handler.php b/tests/php/test-class-amp-tiktok-embed-handler.php index 0f431b69548..35b2e137fa5 100644 --- a/tests/php/test-class-amp-tiktok-embed-handler.php +++ b/tests/php/test-class-amp-tiktok-embed-handler.php @@ -82,18 +82,13 @@ public function get_conversion_data() { 'https://www.tiktok.com/@scout2015/video/6718335390845095173' . PHP_EOL, ' - -
- @scout2015 -

Scramble up ur name & I’ll try to guess it😍❤️ #foryoupage - #petsoftiktok - #aesthetic -

-

- ♬ original sound – tiff -

-
- + +
+
@scout2015 +

Scramble up ur name & I’ll try to guess it😍❤️ #foryoupage #petsoftiktok #aesthetic

+

♬ original sound – tiff

+
+ ', ], @@ -102,13 +97,15 @@ public function get_conversion_data() {
@countingprimes

You can now embed TikTok\'s in AMP

♬ original sound - countingprimes
', ' - -
- @countingprimes -

You can now embed TikTok’s in AMP

-

♬ original sound – countingprimes

-
- + +
+
+ @countingprimes +

You can now embed TikTok’s in AMP

+

♬ original sound – countingprimes

+
+
+ ', ], From bbcbad3be3c38b06c0db5b1f60dc9fd5365796b3 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 24 Aug 2021 13:21:14 -0700 Subject: [PATCH 3/6] Simply check for removing script wrapped in wpautop --- includes/embeds/class-amp-tiktok-embed-handler.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/includes/embeds/class-amp-tiktok-embed-handler.php b/includes/embeds/class-amp-tiktok-embed-handler.php index bee3c8febfd..d7c33e77024 100644 --- a/includes/embeds/class-amp-tiktok-embed-handler.php +++ b/includes/embeds/class-amp-tiktok-embed-handler.php @@ -94,8 +94,12 @@ protected function remove_embed_script( Element $blockquote ) { // Handle case where script is wrapped in paragraph by wpautop. if ( $next_element_sibling instanceof Element && Tag::P === $next_element_sibling->nodeName ) { - $children = $next_element_sibling->getElementsByTagName( '*' ); - if ( 1 === $children->length && Tag::SCRIPT === $children->item( 0 )->nodeName && false !== strpos( $children->item( 0 )->getAttribute( Attribute::SRC ), $script_src ) ) { + $script = $next_element_sibling->getElementsByTagName( Tag::SCRIPT )->item( 0 ); + if ( + $script instanceof Element + && + false !== strpos( $script->getAttribute( Attribute::SRC ), $script_src ) + ) { $next_element_sibling->parentNode->removeChild( $next_element_sibling ); return; } From e6949f45ecb830beb8b61a79787826637640a766 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 24 Aug 2021 21:28:24 -0700 Subject: [PATCH 4/6] Use Extension::TIKTOK in XPath Co-authored-by: Pierre Gordon <16200219+pierlon@users.noreply.github.com> --- includes/embeds/class-amp-tiktok-embed-handler.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/includes/embeds/class-amp-tiktok-embed-handler.php b/includes/embeds/class-amp-tiktok-embed-handler.php index d7c33e77024..f399483ab51 100644 --- a/includes/embeds/class-amp-tiktok-embed-handler.php +++ b/includes/embeds/class-amp-tiktok-embed-handler.php @@ -39,7 +39,9 @@ public function unregister_embed() { * @param Document $dom DOM. */ public function sanitize_raw_embeds( Document $dom ) { - $nodes = $dom->xpath->query( '//blockquote[ @cite and @data-video-id and contains( @class, "tiktok-embed" ) and not( parent::amp-tiktok ) ]' ); + $nodes = $dom->xpath->query( + sprintf( '//blockquote[ @cite and @data-video-id and contains( @class, "tiktok-embed" ) and not( parent::%s ) ]', Extension::TIKTOK ) + ); foreach ( $nodes as $node ) { $this->make_embed_amp_compatible( $node ); From dd3eec2c922fe2e4290c225a04db29705f98de28 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 24 Aug 2021 22:03:18 -0700 Subject: [PATCH 5/6] Update skip message for TikTok oEmbed conversion Co-authored-by: Pierre Gordon <16200219+pierlon@users.noreply.github.com> --- tests/php/test-class-amp-tiktok-embed-handler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/test-class-amp-tiktok-embed-handler.php b/tests/php/test-class-amp-tiktok-embed-handler.php index 35b2e137fa5..b83c3caf9c8 100644 --- a/tests/php/test-class-amp-tiktok-embed-handler.php +++ b/tests/php/test-class-amp-tiktok-embed-handler.php @@ -138,7 +138,7 @@ public function get_conversion_data() { */ public function test_conversion( $source, $expected = null ) { if ( version_compare( '5.4-alpha', get_bloginfo( 'version' ), '>' ) ) { - $this->markTestSkipped( 'The TikTok embed is only available in 5.4-alpha (until 5.4 is stable)' ); + $this->markTestSkipped( 'The TikTok oEmbed provider is only available in 5.4-alpha and later' ); } if ( ! $expected ) { $expected = $source; From 4d03ae738797bdef874c68eef95c42f67f5792d8 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 24 Aug 2021 22:17:12 -0700 Subject: [PATCH 6/6] Test TikTok embed in Custom HTML block to fix missing coverage --- .../test-class-amp-tiktok-embed-handler.php | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/php/test-class-amp-tiktok-embed-handler.php b/tests/php/test-class-amp-tiktok-embed-handler.php index b83c3caf9c8..c7fdd8689db 100644 --- a/tests/php/test-class-amp-tiktok-embed-handler.php +++ b/tests/php/test-class-amp-tiktok-embed-handler.php @@ -73,12 +73,12 @@ public function mock_http_request( $pre, $r, $url ) { */ public function get_conversion_data() { return [ - 'no_embed' => [ + 'no_embed' => [ '

Hello world.

', '

Hello world.

' . PHP_EOL, ], - 'url_simple' => [ + 'url_simple' => [ 'https://www.tiktok.com/@scout2015/video/6718335390845095173' . PHP_EOL, ' @@ -92,7 +92,7 @@ public function get_conversion_data() { ', ], - 'amp-tiktok-embed-code' => [ + 'tiktok-embed-code-with-wpautop' => [ '
@countingprimes

You can now embed TikTok\'s in AMP

♬ original sound - countingprimes
', @@ -109,7 +109,26 @@ public function get_conversion_data() { ', ], - 'amp-tiktok-passthrough' => [ + 'tiktok-embed-code-without-wpautop' => [ + ' + +
@countingprimes

You can now embed TikTok\'s in AMP

♬ original sound - countingprimes
+ + ', + ' + +
+
+ @countingprimes +

You can now embed TikTok’s in AMP

+ ♬ original sound – countingprimes +
+
+ + ', + ], + + 'amp-tiktok-passthrough' => [ '