-
Notifications
You must be signed in to change notification settings - Fork 385
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
#841 Improve support for default embeds #889
Changes from 9 commits
2fda05e
754fe97
657b227
807a84e
941a858
5c33545
eefdb70
6c9079f
1d7cacd
35a62f4
e2cc3e1
e1b74cd
634392b
509b74e
c6d6258
c2efa47
708e577
be133bf
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,70 @@ | ||
<?php | ||
/** | ||
* Class AMP_Issuu_Embed_Handler | ||
* | ||
* @package AMP | ||
* @since 0.7 | ||
*/ | ||
|
||
/** | ||
* Class AMP_Issuu_Embed_Handler | ||
*/ | ||
class AMP_Issuu_Embed_Handler extends AMP_Base_Embed_Handler { | ||
/** | ||
* Regex matched to produce output amp-iframe. | ||
* | ||
* @const string | ||
*/ | ||
const URL_PATTERN = '#https?://(www\.)?issuu\.com/.+/docs/.+#i'; | ||
|
||
/** | ||
* Register embed. | ||
*/ | ||
public function register_embed() { | ||
wp_embed_register_handler( 'issuu', self::URL_PATTERN, array( $this, 'oembed' ) ); | ||
} | ||
|
||
/** | ||
* Unregister embed. | ||
*/ | ||
public function unregister_embed() { | ||
wp_embed_unregister_handler( 'issuu' ); | ||
} | ||
|
||
/** | ||
* Embed found with matching URL callback. | ||
* | ||
* @param array $matches URL regex matches. | ||
* @param array $attr Additional parameters. | ||
* @param array $url URL. | ||
*/ | ||
public function oembed( $matches, $attr, $url ) { | ||
return $this->render( array( 'url' => $url ) ); | ||
} | ||
|
||
/** | ||
* Output the Issuu iframe. | ||
* | ||
* @param array $args parameters used for output. | ||
*/ | ||
public function render( $args ) { | ||
$args = wp_parse_args( $args, array( | ||
'url' => false, | ||
) ); | ||
|
||
if ( empty( $args['url'] ) ) { | ||
return ''; | ||
} | ||
|
||
return AMP_HTML_Utils::build_tag( | ||
'amp-iframe', | ||
array( | ||
'width' => $this->args['width'], | ||
'height' => $this->args['height'], | ||
'src' => $args['url'], | ||
'sandbox' => 'allow-scripts allow-same-origin', | ||
) | ||
); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
<?php | ||
/** | ||
* Class AMP_MeetUp_Embed_Handler | ||
* | ||
* @package AMP | ||
* @since 0.7 | ||
*/ | ||
|
||
/** | ||
* Class AMP_MeetUp_Embed_Handler | ||
*/ | ||
class AMP_MeetUp_Embed_Handler extends AMP_Base_Embed_Handler { | ||
/** | ||
* Regex matched. | ||
* | ||
* @const string | ||
*/ | ||
const URL_PATTERN = '#https?://(www\.)?meetu(\.ps|p\.com)/.*#i'; | ||
|
||
/** | ||
* Register embed. | ||
*/ | ||
public function register_embed() { | ||
wp_embed_register_handler( 'meetup', self::URL_PATTERN, array( $this, 'oembed' ) ); | ||
} | ||
|
||
/** | ||
* Unregister embed. | ||
*/ | ||
public function unregister_embed() { | ||
wp_embed_unregister_handler( 'meetup' ); | ||
} | ||
|
||
/** | ||
* Embed found with matching URL callback. | ||
* | ||
* @param array $matches URL regex matches. | ||
* @param array $attr Additional parameters. | ||
* @param array $url URL. | ||
*/ | ||
public function oembed( $matches, $attr, $url ) { | ||
return $this->render( array( 'url' => $url ) ); | ||
} | ||
|
||
/** | ||
* Extract the MeetUp CSS and output in header (otherwise stripped). | ||
* Output the MeetUp oembed as usual. | ||
* | ||
* @param array $args parameters used for output. | ||
*/ | ||
public function render( $args ) { | ||
$args = wp_parse_args( $args, array( | ||
'url' => false, | ||
) ); | ||
|
||
if ( empty( $args['url'] ) ) { | ||
return ''; | ||
} | ||
|
||
$content = wp_oembed_get( $args['url'] ); | ||
preg_match( '#<style.+</style>#', $content, $result ); | ||
|
||
// Outlying style tag from oembed. | ||
if ( isset( $result[0] ) ) { | ||
$css = str_replace( '<style type="text/css">', '<style amp-meetup>', $result[0] ); | ||
echo $css; // WPCS: XSS ok. | ||
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. In the builtin paired mode theme, the CSS is getting printed before the 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. One additional reason why the CSS should be discarded is that it includes |
||
} | ||
|
||
return $content; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
<?php | ||
/** | ||
* Class AMP_Reddit_Embed_Handler | ||
* | ||
* @package AMP | ||
* @since 0.7 | ||
*/ | ||
|
||
/** | ||
* Class AMP_Reddit_Embed_Handler | ||
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. Please add |
||
*/ | ||
class AMP_Reddit_Embed_Handler extends AMP_Base_Embed_Handler { | ||
/** | ||
* Regex matched to produce output amp-reddit. | ||
* | ||
* @const string | ||
*/ | ||
const URL_PATTERN = '#https?://(www\.)?reddit\.com/r/[^/]+/comments/.*#i'; | ||
|
||
/** | ||
* Register embed. | ||
*/ | ||
public function register_embed() { | ||
wp_embed_register_handler( 'amp-reddit', self::URL_PATTERN, array( $this, 'oembed' ) ); | ||
} | ||
|
||
/** | ||
* Unregister embed. | ||
*/ | ||
public function unregister_embed() { | ||
wp_embed_unregister_handler( 'amp-reddit' ); | ||
} | ||
|
||
/** | ||
* Embed found with matching URL callback. | ||
* | ||
* @param array $matches URL regex matches. | ||
* @param array $attr Additional parameters. | ||
* @param array $url URL. | ||
*/ | ||
public function oembed( $matches, $attr, $url ) { | ||
return $this->render( array( 'url' => $url ) ); | ||
} | ||
|
||
/** | ||
* Output the Reddit amp element. | ||
* | ||
* @param array $args parameters used for output. | ||
*/ | ||
public function render( $args ) { | ||
$args = wp_parse_args( $args, array( | ||
'url' => false, | ||
) ); | ||
|
||
if ( empty( $args['url'] ) ) { | ||
return ''; | ||
} | ||
|
||
return AMP_HTML_Utils::build_tag( | ||
'amp-reddit', | ||
array( | ||
'layout' => 'responsive', | ||
'data-embedtype' => 'post', | ||
'width' => '100', | ||
'height' => '100', | ||
'data-src' => $args['url'], | ||
) | ||
); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
<?php | ||
/** | ||
* Class AMP_Tumblr_Embed_Handler | ||
* | ||
* @package AMP | ||
* @since 0.7 | ||
*/ | ||
|
||
/** | ||
* Class AMP_Tumblr_Embed_Handler | ||
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. Please add |
||
*/ | ||
class AMP_Tumblr_Embed_Handler extends AMP_Base_Embed_Handler { | ||
/** | ||
* Regex matched to produce output amp-iframe. | ||
* | ||
* @const string | ||
*/ | ||
const URL_PATTERN = '#https?://(.+)\.tumblr\.com/post/(.*)#i'; | ||
|
||
/** | ||
* Register embed. | ||
*/ | ||
public function register_embed() { | ||
wp_embed_register_handler( 'tumblr', self::URL_PATTERN, array( $this, 'oembed' ) ); | ||
} | ||
|
||
/** | ||
* Unregister embed. | ||
*/ | ||
public function unregister_embed() { | ||
wp_embed_unregister_handler( 'tumblr' ); | ||
} | ||
|
||
/** | ||
* Embed found with matching URL callback. | ||
* | ||
* @param array $matches URL regex matches. | ||
* @param array $attr Additional parameters. | ||
* @param array $url URL. | ||
*/ | ||
public function oembed( $matches, $attr, $url ) { | ||
return $this->render( array( 'url' => $url ) ); | ||
} | ||
|
||
/** | ||
* Output the Tumblr iframe. | ||
* | ||
* @param array $args parameters used for output. | ||
*/ | ||
public function render( $args ) { | ||
$args = wp_parse_args( $args, array( | ||
'url' => false, | ||
) ); | ||
|
||
if ( empty( $args['url'] ) ) { | ||
return ''; | ||
} | ||
|
||
// Must enforce https for amp-iframe, but editors can supply either on desktop. | ||
$args['url'] = str_replace( 'http://', 'https://', $args['url'] ); | ||
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. Unfortunately it seems this is not working fully. When attempting to embed https://ifpaintingscouldtext.tumblr.com/post/92003045635/grant-wood-american-gothic-1930 then Tumblr will redirect from HTTPS to HTTP for some reason. When serving a page over HTTPS, as they should be, then the iframe is blocked from rendering. |
||
|
||
return AMP_HTML_Utils::build_tag( | ||
'amp-iframe', | ||
array( | ||
'width' => $this->args['width'], | ||
'height' => $this->args['height'], | ||
'src' => $args['url'], | ||
) | ||
); | ||
} | ||
} | ||
|
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.
I'm seeing that
wp_oembed_get()
here is resulting in an HTTP request with every call. I was expecting it to be cached. Apparently to cache we need to use$wp_embed->shortcode( array(), $args['url'] )
. This seems to be a problem in core as well with the Video widget: https://github.com/WordPress/wordpress-develop/blob/afb4f45e62520397b147e560e051d3940797121e/src/wp-includes/widgets/class-wp-widget-media-video.php#L144I think core is erroneously doing an HTTP request for every load of a page with a video widget that is a non-YouTube and non-Vimeo oEmbed.