-
Notifications
You must be signed in to change notification settings - Fork 384
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
Add embed handler for gfycat. #1136
Changes from 4 commits
ca88c0c
e295c81
9cad8e6
46edf1e
e1d29a8
db570af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
<?php | ||
/** | ||
* Class AMP_Gfycat_Embed_Handler | ||
* | ||
* @package AMP | ||
* @since 1.0 | ||
*/ | ||
|
||
/** | ||
* Class AMP_Gfycat_Embed_Handler | ||
*/ | ||
class AMP_Gfycat_Embed_Handler extends AMP_Base_Embed_Handler { | ||
/** | ||
* Regex matched to produce output amp-gfycat. | ||
* | ||
* @var string | ||
*/ | ||
const URL_PATTERN = '#https?://(www\.)?gfycat\.com/gifs/detail/.*#i'; | ||
|
||
/** | ||
* Register embed. | ||
*/ | ||
public function register_embed() { | ||
add_filter( 'embed_oembed_html', array( $this, 'filter_embed_oembed_html' ), 10, 3 ); | ||
} | ||
|
||
/** | ||
* Unregister embed. | ||
*/ | ||
public function unregister_embed() { | ||
remove_filter( 'embed_oembed_html', array( $this, 'filter_embed_oembed_html' ), 10 ); | ||
} | ||
|
||
/** | ||
* Filter oEmbed HTML for Gfycat to prepare it for AMP. | ||
* | ||
* @param mixed $return The shortcode callback function to call. | ||
* @param string $url The attempted embed URL. | ||
* @param array $attr An array of shortcode attributes. | ||
* @return string Embed. | ||
*/ | ||
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]; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this bail if the width and height are empty? In that case we wouldn't be able to build a valid There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added checking for |
||
|
||
$pieces = explode( '/detail/', $parsed_url['path'] ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't taking into account the oEmbed URLs like https://gfycat.com/webbedbossycollardlizard |
||
if ( ! isset( $pieces[1] ) ) { | ||
return $return; | ||
} | ||
|
||
$data_gfyid = $pieces[1]; | ||
|
||
$return = AMP_HTML_Utils::build_tag( | ||
'amp-gfycat', | ||
array( | ||
'width' => $attr['width'], | ||
'height' => $attr['height'], | ||
'data-gfyid' => $data_gfyid, | ||
'layout' => 'intrinsic', | ||
) | ||
); | ||
} | ||
return $return; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
<?php | ||
/** | ||
* Test Gfycat embed. | ||
* | ||
* @package AMP. | ||
*/ | ||
|
||
/** | ||
* Class AMP_Gfycat_Embed_Test | ||
*/ | ||
class AMP_Gfycat_Embed_Test extends WP_UnitTestCase { | ||
|
||
/** | ||
* Set up. | ||
* | ||
* @global WP_Post $post | ||
*/ | ||
public function setUp() { | ||
global $post; | ||
parent::setUp(); | ||
|
||
/* | ||
* As #34115 in 4.9 a post is not needed for context to run oEmbeds. Prior ot 4.9, the WP_Embed::shortcode() | ||
* method would short-circuit when this is the case: | ||
* https://github.com/WordPress/wordpress-develop/blob/4.8.4/src/wp-includes/class-wp-embed.php#L192-L193 | ||
* So on WP<4.9 we set a post global to ensure oEmbeds get processed. | ||
*/ | ||
if ( version_compare( strtok( get_bloginfo( 'version' ), '-' ), '4.9', '<' ) ) { | ||
$post = $this->factory()->post->create_and_get(); | ||
} | ||
} | ||
|
||
/** | ||
* Get conversion data. | ||
* | ||
* @return array | ||
*/ | ||
public function get_conversion_data() { | ||
return array( | ||
'no_embed' => array( | ||
'<p>Hello world.</p>', | ||
'<p>Hello world.</p>' . PHP_EOL, | ||
), | ||
|
||
'url_simple' => array( | ||
'https://gfycat.com/gifs/detail/tautwhoppingcougar' . PHP_EOL, | ||
'<p><amp-gfycat width="500" height="750" data-gfyid="tautwhoppingcougar" layout="intrinsic"></amp-gfycat></p>' . PHP_EOL, | ||
), | ||
|
||
); | ||
} | ||
|
||
/** | ||
* Test conversion. | ||
* | ||
* @param string $source Source. | ||
* @param string $expected Expected. | ||
* @dataProvider get_conversion_data | ||
*/ | ||
public function test__conversion( $source, $expected ) { | ||
$embed = new AMP_Gfycat_Embed_Handler(); | ||
$embed->register_embed(); | ||
$filtered_content = apply_filters( 'the_content', $source ); | ||
|
||
$this->assertEquals( $expected, $filtered_content ); | ||
} | ||
|
||
/** | ||
* Get scripts data. | ||
* | ||
* @return array | ||
*/ | ||
public function get_scripts_data() { | ||
return array( | ||
'not_converted' => array( | ||
'<p>Hello World.</p>', | ||
array(), | ||
), | ||
'converted' => array( | ||
'https://www.gfycat.com/gifs/detail/tautwhoppingcougar' . PHP_EOL, | ||
array( 'amp-gfycat' => true ), | ||
), | ||
); | ||
} | ||
|
||
/** | ||
* Test scripts. | ||
* | ||
* @param string $source Source. | ||
* @param string $expected Expected. | ||
* @dataProvider get_scripts_data | ||
*/ | ||
public function test__get_scripts( $source, $expected ) { | ||
$embed = new AMP_Gfycat_Embed_Handler(); | ||
$embed->register_embed(); | ||
$source = apply_filters( 'the_content', $source ); | ||
|
||
$whitelist_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( AMP_DOM_Utils::get_dom_from_content( $source ) ); | ||
$whitelist_sanitizer->sanitize(); | ||
|
||
$scripts = array_merge( | ||
$embed->get_scripts(), | ||
$whitelist_sanitizer->get_scripts() | ||
); | ||
|
||
$this->assertEquals( $expected, $scripts ); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to adjust this to be like
'/width=["\']?(\d+)/'
since the quote marks would be optional in HTML.