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.

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

Improve handling of Tumblr embeds #5926

Merged
merged 28 commits into from
Mar 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9de2cfc
Sanitize Tumblr embeds
pierlon Feb 25, 2021
a414ddf
Fix lint errors
pierlon Feb 25, 2021
ebd31df
Add test for `https://teded.tumblr.com/post/184736320764/how-do-vacci…
pierlon Feb 25, 2021
9125cd6
Use `tagName` instead of `nodeName`
pierlon Feb 25, 2021
ca74ffa
Remove escaping of text
pierlon Feb 25, 2021
da37307
Make use of tagName, use constants where possible, add phpdoc
westonruter Feb 25, 2021
1475347
Check if `tumblr-post` is apart of the list of classes
pierlon Feb 25, 2021
c330597
Use callback to determine if script should be removed
pierlon Feb 26, 2021
7df8bbe
Use button as overflow element
pierlon Feb 26, 2021
3323625
Add bottom:0 to Tumblr overflow button
westonruter Feb 26, 2021
8ae2574
Make xpath predicate more specific and improve condition formatting
westonruter Mar 3, 2021
005ea45
Remove extraneous parentheses
westonruter Mar 3, 2021
5c4b667
Apply style rules for AMP iframe resize button via core sanitizer
pierlon Mar 3, 2021
6d4bef6
Fix indentation
pierlon Mar 3, 2021
f56f10c
Set button type
pierlon Mar 3, 2021
2740d7e
Add style rule to place overflow button in bottom left corner of AMP …
pierlon Mar 4, 2021
7da8a35
Make sure TT1 stylesheet is enqueued before adding inline style
westonruter Mar 4, 2021
f642be8
Opt to combine add_twentytwentyone_overflow_button_fix into amend_twe…
westonruter Mar 4, 2021
0c6666e
Make sure that Tumblr embed has a link to use as a placeholder
westonruter Mar 4, 2021
685bdc9
Make use of MarkupComparison
westonruter Mar 4, 2021
435b6ad
Test removal of Tumblr script when wpautop was not involved
westonruter Mar 4, 2021
bcbdf00
Add test for tumblr in embed block
westonruter Mar 4, 2021
8fa44dd
Remove extraneous sprintf()
westonruter Mar 4, 2021
c482fcb
Combine conditions
westonruter Mar 4, 2021
ce2b8b4
Use xpath query to obtain the placeholder
westonruter Mar 4, 2021
77c7367
Set placeholder attribute when we are sure we will match
westonruter Mar 4, 2021
8839cc1
Remove extraneous sprintf()
westonruter Mar 4, 2021
ad2e9be
Fix tests in WP 4.9
westonruter Mar 4, 2021
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
8 changes: 8 additions & 0 deletions assets/css/src/amp-default.css
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,11 @@ amp-carousel .amp-wp-gallery-caption {
margin-bottom: 1.5em;
max-width: 100%;
}

/*
* Ensure the button used to expand AMP components is placed in the bottom left hand corner of the component,
* where it's most likely to be seen.
*/
button[overflow] {
bottom: 0;
}
49 changes: 49 additions & 0 deletions includes/embeds/class-amp-base-embed-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* @package AMP
*/

use AmpProject\Tag;

/**
* Class AMP_Base_Embed_Handler
*
Expand Down Expand Up @@ -151,4 +153,51 @@ protected function unwrap_p_element( DOMElement $node ) {
$parent_node->parentNode->replaceChild( $node, $parent_node );
}
}

/**
* Removes the node's nearest `<script>` sibling with a `src` attribute containing the base `src` URL provided.
*
* @since 2.1
*
* @param DOMElement $node The DOMNode to whose sibling is the script to be removed.
* @param callable $match_callback Callback which is passed the script element to determine if it is a match.
*/
protected function maybe_remove_script_sibling( DOMElement $node, callable $match_callback ) {
$next_element_sibling = $node->nextSibling;
while ( $next_element_sibling && ! $next_element_sibling instanceof DOMElement ) {
$next_element_sibling = $next_element_sibling->nextSibling;
}
if ( ! $next_element_sibling instanceof DOMElement ) {
return;
}

// Handle case where script is immediately following.
if ( Tag::SCRIPT === $next_element_sibling->tagName && $match_callback( $next_element_sibling ) ) {
$next_element_sibling->parentNode->removeChild( $next_element_sibling );
return;
}

// Handle case where script is wrapped in paragraph by wpautop.
if ( 'p' === $next_element_sibling->tagName ) {
/** @var DOMElement[] $children_elements */
$children_elements = array_values(
array_filter(
iterator_to_array( $next_element_sibling->childNodes ),
static function ( DOMNode $child ) {
return $child instanceof DOMElement;
}
)
);

if (
1 === count( $children_elements )
&&
Tag::SCRIPT === $children_elements[0]->tagName
&&
$match_callback( $children_elements[0] )
) {
$next_element_sibling->parentNode->removeChild( $next_element_sibling );
}
}
}
}
100 changes: 77 additions & 23 deletions includes/embeds/class-amp-tumblr-embed-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,56 +6,110 @@
* @since 0.7
*/

use AmpProject\Attribute;
use AmpProject\Dom\Document;
use AmpProject\Tag;

/**
* Class AMP_Tumblr_Embed_Handler
*
* @internal
*/
class AMP_Tumblr_Embed_Handler extends AMP_Base_Embed_Handler {

/**
* Default width.
*
* Tumblr embeds for web have a fixed width of 540px.
*
* @link https://tumblr.zendesk.com/hc/en-us/articles/226261028-Embed-pro-tips
* @var int
*/
protected $DEFAULT_WIDTH = 540;

/**
* Register embed.
*/
public function register_embed() {
add_filter( 'embed_oembed_html', [ $this, 'filter_embed_oembed_html' ], 10, 2 );
// Not implemented.
}

/**
* Unregister embed.
*/
public function unregister_embed() {
remove_filter( 'embed_oembed_html', [ $this, 'filter_embed_oembed_html' ], 10 );
// Not implemented.
}

/**
* Filter oEmbed HTML for Tumblr to prepare it for AMP.
* Sanitizes Tumblr raw embeds to make them AMP compatible.
*
* @param string $cache Cache for oEmbed.
* @param string $url Embed URL.
* @return string Embed.
* @param Document $dom DOM.
*/
public function filter_embed_oembed_html( $cache, $url ) {
$parsed_url = wp_parse_url( $url );
if ( false === strpos( $parsed_url['host'], 'tumblr.com' ) ) {
return $cache;
public function sanitize_raw_embeds( Document $dom ) {
$placeholders = $dom->xpath->query(
'//div[ @class and @data-href and contains( concat( " ", normalize-space( @class ), " " ), " tumblr-post " ) and starts-with( @data-href, "https://embed.tumblr.com/embed/post/" ) ]/a[ @href ]'
);

if ( 0 === $placeholders->length ) {
return;
}

// @todo The iframe will not get sized properly.
if ( preg_match( '#data-href="(?P<href>https://embed.tumblr.com/embed/post/\w+/\w+)"#', $cache, $matches ) ) {
$cache = AMP_HTML_Utils::build_tag(
foreach ( $placeholders as $placeholder ) {
$div = $placeholder->parentNode;
if ( ! $div instanceof DOMElement ) {
continue; // @codeCoverageIgnore
}

$attributes = [
'src' => $div->getAttribute( 'data-href' ),
'layout' => 'responsive',
'width' => $this->args['width'],
'height' => $this->args['height'],
'resizable' => '',
'sandbox' => 'allow-scripts allow-popups allow-same-origin',
];

$amp_element = AMP_DOM_Utils::create_node(
$dom,
'amp-iframe',
$attributes
);

// Add an overflow element to allow the amp-iframe to be manually resized.
$overflow_element = AMP_DOM_Utils::create_node(
$dom,
'button',
[
'width' => $this->args['width'],
'height' => $this->args['height'],
'layout' => 'responsive',
'sandbox' => 'allow-scripts allow-popups', // The allow-scripts is needed to allow the iframe to render; allow-popups needed to allow clicking.
'src' => $matches['href'],
],
sprintf( '<a placeholder href="%s">Tumblr</a>', $url )
'overflow' => '',
pierlon marked this conversation as resolved.
Show resolved Hide resolved
'type' => 'button',
]
);
}
$overflow_element->textContent = __( 'See more', 'amp' );
$amp_element->appendChild( $overflow_element );
westonruter marked this conversation as resolved.
Show resolved Hide resolved
$placeholder->setAttribute( Attribute::PLACEHOLDER, '' );
$amp_element->appendChild( $placeholder );

$this->maybe_remove_script_sibling(
$div,
static function ( DOMElement $script ) {
if ( ! $script->hasAttribute( Attribute::SRC ) ) {
return false;
}

$parsed_url = wp_parse_url( $script->getAttribute( Attribute::SRC ) );

return $cache;
return (
isset( $parsed_url['host'], $parsed_url['path'] )
&&
'assets.tumblr.com' === $parsed_url['host']
&&
'/post.js' === $parsed_url['path']
);
}
);

$div->parentNode->replaceChild( $amp_element, $div );
}
}
}

31 changes: 24 additions & 7 deletions includes/sanitizers/class-amp-core-theme-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -2012,12 +2012,19 @@ public static function amend_twentytwentyone_dark_mode_styles() {
add_action(
'wp_enqueue_scripts',
static function() {
// Bail if the dark mode stylesheet is not enqueued.
if ( ! wp_style_is( 'tt1-dark-mode' ) ) {
$theme_style_handle = 'twenty-twenty-one-style';
$dark_mode_style_handle = 'tt1-dark-mode';

// Bail if the dark mode stylesheet is not enqueued or the theme stylesheet isn't registered.
if (
! wp_style_is( $dark_mode_style_handle, 'enqueued' )
||
! wp_style_is( $theme_style_handle, 'registered' )
) {
return; // @codeCoverageIgnore
}

wp_dequeue_style( 'tt1-dark-mode' );
wp_dequeue_style( $dark_mode_style_handle );

$dark_mode_css_file = get_theme_file_path(
sprintf( 'assets/css/style-dark-mode%s.css', is_rtl() ? '-rtl' : '' )
Expand All @@ -2035,15 +2042,14 @@ static function() {
$new_styles = str_replace( '.is-dark-theme.is-dark-theme', ':root', $new_styles );
$new_styles = str_replace( '.respect-color-scheme-preference.is-dark-theme body', '.respect-color-scheme-preference body', $new_styles );

wp_add_inline_style( 'twenty-twenty-one-style', $new_styles );
wp_add_inline_style( $theme_style_handle, $new_styles );
},
11
);
}

/**
* Amend the Twenty Twenty-One stylesheet to make it compatible with the changes made to the document during
* sanitization.
* Amend Twenty Twenty-One Styles.
*/
public static function amend_twentytwentyone_styles() {
add_action(
Expand Down Expand Up @@ -2072,7 +2078,7 @@ static function() {

$styles = file_get_contents( $css_file ); //phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents

// Append any extra rules that may be needed.
// Append extra rules needed for nav menus according to changes made to the document during sanitization.
$styles .= '
/* Trap keyboard navigation within mobile menu when it\'s open */
@media only screen and (max-width: 481px) {
Expand Down Expand Up @@ -2103,6 +2109,17 @@ static function() {
}
';

/*
* In Twenty Twenty-One, when a button is used to resize AMP elements, they can appear transparent when hovered over.
* To resolve this issue, the theme's background color is used as the background color for the button
* when it is in the hovered state.
*/
$styles .= '
button[overflow]:hover {
background-color: var(--global--color-background);
}
';

// Ideally wp_add_inline_style() would accept a $position argument like wp_add_inline_script() does, in
// which case the following could be replaced with wp_add_inline_style( $style_handle, $new_styles, 'before' ).
// But this is not supported, so we have to resort to manipulating the underlying after array.
Expand Down
Loading