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

Unwrap all embeds with paragraph tags #4650

Closed
wants to merge 51 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
7401840
Sanitize DailyMotion embeds
pierlon Apr 1, 2020
3c57f35
Sanitize gfycat embeds
pierlon Apr 27, 2020
7e1ccf0
Unwrap Instagram embeds
pierlon Apr 27, 2020
3bb9d3e
Unwrap Scribd embeds
pierlon May 4, 2020
be260f8
Unwrap SoundCloud embeds
pierlon May 5, 2020
d7f3421
Sanitize Twitter timelines and moments
pierlon May 8, 2020
9c17878
Unwrap Vimeo embeds
pierlon May 8, 2020
91cdd3d
Sanitize YouTube embeds
pierlon May 8, 2020
ce5ef79
Sanitize WordPress TV embeds
pierlon May 8, 2020
575f173
Sanitize Hulu embeds
pierlon May 8, 2020
d789507
Fix lint errors
pierlon May 8, 2020
2ee90b4
Merge branch 'develop' into fix/4450-wrapped-embeds
pierlon May 16, 2020
bd606bf
Make child element count more robust
pierlon May 18, 2020
8e492b2
Extract base embed URLs as class constants
pierlon May 18, 2020
3f4f9ae
Get dimensions from iframe if available
pierlon May 18, 2020
085d1cc
Refactor logic for removing script sibling
pierlon May 18, 2020
050b9e7
Simplify logic for retrieving embed IDs
pierlon May 18, 2020
6893a47
Fix tests
pierlon May 19, 2020
34ca29c
Implement the Template Method design pattern
pierlon May 21, 2020
c955f45
Sanitize Imgur embeds
pierlon May 21, 2020
21ae657
Sanitize Tumblr embeds
pierlon May 21, 2020
8850a0f
Mock tests for DailyMotion
pierlon May 21, 2020
decbb49
Sanitize Reddit embeds
pierlon May 22, 2020
361b712
Implement `get_raw_embed_nodes` in `AMP_Base_Embed_Handler`
pierlon May 22, 2020
28b4394
Remove `get_parent_element` method
pierlon May 22, 2020
c663535
Make YouTube xpath query more specific
pierlon Jun 5, 2020
1f4df49
Merge branch 'develop' into fix/4450-wrapped-embeds
pierlon Jun 17, 2020
573334a
Fix lint issues
pierlon Jun 18, 2020
c695846
Mock Instagram embed test
pierlon Jun 18, 2020
67bf708
Update docblocks specifying alternative return values
pierlon Jun 18, 2020
e1c768d
Fix test cases for Dailymotion, Twitter, Gfycat and WordPress TV embeds
pierlon Jun 19, 2020
5b2ece9
Update mocked responses
pierlon Jun 19, 2020
5fa1cdb
Sanitize Meetup embeds
pierlon Jun 19, 2020
85b55cb
Sanitize Issuu embeds
pierlon Jun 19, 2020
996862a
Sanitize CrowdSignal embeds
pierlon Jun 19, 2020
1cd716f
Merge branch 'develop' into fix/4450-wrapped-embeds
pierlon Jun 19, 2020
4885788
Sanitize Pinterest embeds
pierlon Jun 19, 2020
59066e8
Sanitize Vine embeds
pierlon Jun 21, 2020
7c03ced
Add `Registerable` interface to indicate embed handlers that hook int…
pierlon Jun 21, 2020
1d91d0b
Fix lint and static analysis errors
pierlon Jun 21, 2020
cb3ef8b
Merge branch 'develop' into fix/4450-wrapped-embeds
pierlon Jun 21, 2020
beadf51
Rename `whitelist_sanitizer` to `validating_sanitizer`
pierlon Jun 21, 2020
b0157a1
Mark Pinterest and Vine embed handlers as registerable
pierlon Jun 25, 2020
b3a2c1c
Add oEmbed provider URLs for embeds not supported in WP 5.1
pierlon Jun 25, 2020
dfaf3e5
Fix YouTube embed test
pierlon Jun 25, 2020
244b53d
Merge branch 'develop' into fix/4450-wrapped-embeds
pierlon Jun 25, 2020
a857f25
Remove unused vars
pierlon Jun 25, 2020
434660a
Fix tests failures related to Gutenberg
pierlon Jun 25, 2020
986c036
Add oEmbed provider URL for Crowdsignal surveys on WP 5.2.
pierlon Jun 25, 2020
62fa5ae
Merge branch 'develop' of github.com:ampproject/amp-wp into fix/4450-…
westonruter Jun 25, 2020
0860f48
Update phpdoc to reflect where methods do not return null
westonruter Jun 25, 2020
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
17 changes: 17 additions & 0 deletions includes/embeds/class-amp-base-embed-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,21 @@ function ( $attr_name ) {
}
return wp_array_slice_assoc( $matches, $attribute_names );
}

/**
* Replace the node's parent with itself if the parent is a <p> tag, has no attributes and has no other children.
* This usually happens while `wpautop()` processes the element.
*
* @param DOMElement $node Node.
*/
protected function maybe_unwrap_p_element( DOMElement $node ) {
$parent_element = AMP_DOM_Utils::get_parent_element( $node );

if ( $parent_element && 'p' === $parent_element->nodeName && false === $parent_element->hasAttributes() ) {
$children = $parent_element->getElementsByTagName( '*' );
pierlon marked this conversation as resolved.
Show resolved Hide resolved
if ( 1 === $children->length ) {
pierlon marked this conversation as resolved.
Show resolved Hide resolved
$parent_element->parentNode->replaceChild( $node, $parent_element );
}
}
}
}
106 changes: 45 additions & 61 deletions includes/embeds/class-amp-dailymotion-embed-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
* @package AMP
*/

use AmpProject\Dom\Document;

/**
* Class AMP_DailyMotion_Embed_Handler
*
* Much of this class is borrowed from Jetpack embeds
*/
class AMP_DailyMotion_Embed_Handler extends AMP_Base_Embed_Handler {

const URL_PATTERN = '#https?:\/\/(www\.)?dailymotion\.com\/video\/.*#i';
const RATIO = 0.5625;
const RATIO = 0.5625;

/**
* Default width.
Expand Down Expand Up @@ -48,86 +49,69 @@ public function __construct( $args = [] ) {
* Register embed.
*/
public function register_embed() {
wp_embed_register_handler( 'amp-dailymotion', self::URL_PATTERN, [ $this, 'oembed' ], -1 );
// Not implemented.
}

/**
* Unregister embed.
*/
public function unregister_embed() {
wp_embed_unregister_handler( 'amp-dailymotion', -1 );
// Not implemented.
}

/**
* Render oEmbed.
*
* @see \WP_Embed::shortcode()
* Sanitize all DailyMotion <iframe> tags to <amp-dailymotion>.
*
* @param array $matches URL pattern matches.
* @param array $attr Shortcode attributes.
* @param string $url URL.
* @param string $rawattr Unmodified shortcode attributes.
* @return string Rendered oEmbed.
* @param Document $dom DOM.
*/
public function oembed( $matches, $attr, $url, $rawattr ) {
$video_id = $this->get_video_id_from_url( $url );
return $this->render(
[
'video_id' => $video_id,
]
);
public function sanitize_raw_embeds( Document $dom ) {
pierlon marked this conversation as resolved.
Show resolved Hide resolved
$nodes = $dom->xpath->query( '//iframe[ starts-with( @src, "https://www.dailymotion.com/embed/video/" ) ]' );
pierlon marked this conversation as resolved.
Show resolved Hide resolved

foreach ( $nodes as $node ) {
if ( ! $this->is_raw_embed( $node ) ) {
continue;
}
$this->sanitize_raw_embed( $node );
}
}

/**
* Render.
* Determine if the node has already been sanitized.
*
* @param array $args Args.
* @return string Rendered.
* @param DOMElement $node The DOMNode.
* @return bool Whether the node is a raw embed.
*/
public function render( $args ) {
$args = wp_parse_args(
$args,
[
'video_id' => false,
]
);

if ( empty( $args['video_id'] ) ) {
return AMP_HTML_Utils::build_tag(
'a',
[
'href' => esc_url_raw( $args['url'] ),
'class' => 'amp-wp-embed-fallback',
],
esc_html( $args['url'] )
);
}

$this->did_convert_elements = true;

return AMP_HTML_Utils::build_tag(
'amp-dailymotion',
[
'data-videoid' => $args['video_id'],
'layout' => 'responsive',
'width' => $this->args['width'],
'height' => $this->args['height'],
]
);
protected function is_raw_embed( DOMElement $node ) {
return $node->parentNode && 'amp-dailymotion' !== $node->parentNode->nodeName;
}
pierlon marked this conversation as resolved.
Show resolved Hide resolved

/**
* Determine the video ID from the URL.
* Make DailyMotion embed AMP compatible.
*
* @param string $url URL.
* @return integer Video ID.
* @param DOMElement $iframe_node The node to make AMP compatible.
*/
private function get_video_id_from_url( $url ) {
$parsed_url = wp_parse_url( $url );
parse_str( $parsed_url['path'], $path );
$tok = explode( '/', $parsed_url['path'] );
$tok = explode( '_', $tok[2] );
private function sanitize_raw_embed( DOMElement $iframe_node ) {
pierlon marked this conversation as resolved.
Show resolved Hide resolved
$iframe_src = $iframe_node->getAttribute( 'src' );

if ( preg_match( '#video/(?P<video_id>[a-zA-Z0-9]+)#', $iframe_src, $matches ) ) {
pierlon marked this conversation as resolved.
Show resolved Hide resolved
$video_id = $matches['video_id'];

$amp_node = AMP_DOM_Utils::create_node(
Document::fromNode( $iframe_node ),
'amp-dailymotion',
[
'data-videoid' => $video_id,
'layout' => 'responsive',
'width' => $this->args['width'],
'height' => $this->args['height'],
pierlon marked this conversation as resolved.
Show resolved Hide resolved
]
);

$this->maybe_unwrap_p_element( $iframe_node );

$iframe_node->parentNode->replaceChild( $amp_node, $iframe_node );
}

return $tok[0];
// Nothing to be done if the video ID could not be found.
}
}
95 changes: 58 additions & 37 deletions includes/embeds/class-amp-gfycat-embed-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* @since 1.0
*/

use AmpProject\Dom\Document;

/**
* Class AMP_Gfycat_Embed_Handler
*/
Expand All @@ -21,61 +23,80 @@ class AMP_Gfycat_Embed_Handler extends AMP_Base_Embed_Handler {
* Register embed.
*/
public function register_embed() {
add_filter( 'embed_oembed_html', [ $this, 'filter_embed_oembed_html' ], 10, 3 );
// 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 Gfycat to prepare it for AMP.
* Sanitize all gfycat <iframe> tags to <amp-gfycat>.
*
* @param mixed $return The oEmbed HTML.
* @param string $url The attempted embed URL.
* @param array $attr Attributes.
* @return string Embed.
* @param Document $dom DOM.
*/
public function filter_embed_oembed_html( $return, $url, $attr ) {
$parsed_url = wp_parse_url( $url );
if ( false !== strpos( $parsed_url['host'], 'gfycat.com' ) ) {
if ( preg_match( '/width=["\']?(\d+)/', $return, $matches ) ) {
$attr['width'] = $matches[1];
}
if ( preg_match( '/height=["\']?(\d+)/', $return, $matches ) ) {
$attr['height'] = $matches[1];
}
public function sanitize_raw_embeds( Document $dom ) {
$nodes = $dom->xpath->query( '//iframe[ starts-with( @src, "https://gfycat.com/ifr/" ) ]' );
pierlon marked this conversation as resolved.
Show resolved Hide resolved

if ( empty( $attr['height'] ) ) {
return $return;
foreach ( $nodes as $node ) {
if ( ! $this->is_raw_embed( $node ) ) {
continue;
}
$this->sanitize_raw_embed( $node );
}
}

$attributes = wp_array_slice_assoc( $attr, [ 'width', 'height' ] );
/**
* 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->parentNode && 'amp-gfycat' !== $node->parentNode->nodeName;
}

if ( empty( $attr['width'] ) ) {
$attributes['layout'] = 'fixed-height';
$attributes['width'] = 'auto';
}
/**
* Make DailyMotion embed AMP compatible.
*
* @param DOMElement $iframe_node The node to make AMP compatible.
*/
private function sanitize_raw_embed( DOMElement $iframe_node ) {
$iframe_src = $iframe_node->getAttribute( 'src' );

$pieces = explode( '/detail/', $parsed_url['path'] );
if ( ! isset( $pieces[1] ) ) {
if ( ! preg_match( '/\/([A-Za-z0-9]+)/', $parsed_url['path'], $matches ) ) {
return $return;
}
$attributes['data-gfyid'] = $matches[1];
} else {
$attributes['data-gfyid'] = $pieces[1];
}
if ( ! preg_match( '#/ifr/([A-Za-z0-9]+)#', $iframe_src, $matches ) ) {
pierlon marked this conversation as resolved.
Show resolved Hide resolved
// Nothing to do if video ID could not be found.
return;
}

$new_attributes = [
'data-gfyid' => $matches[1],
'layout' => 'responsive',
'height' => $iframe_node->getAttribute( 'height' ),
'width' => $iframe_node->getAttribute( 'width' ),
pierlon marked this conversation as resolved.
Show resolved Hide resolved
];

$return = AMP_HTML_Utils::build_tag(
'amp-gfycat',
$attributes
);
if ( empty( $new_attributes['height'] ) ) {
return;
}
return $return;

if ( empty( $new_attributes['width'] ) ) {
$new_attributes['layout'] = 'fixed-height';
$new_attributes['width'] = 'auto';
}

$amp_node = AMP_DOM_Utils::create_node(
Document::fromNode( $iframe_node ),
'amp-gfycat',
$new_attributes
);

$this->maybe_unwrap_p_element( $iframe_node );

$iframe_node->parentNode->replaceChild( $amp_node, $iframe_node );
}
}

Loading