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

Preserve seek time and other parameters to amp-youtube from YouTube embeds #6423

Merged
merged 43 commits into from
Sep 9, 2021
Merged
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
2b00609
Transfer seek time to AMP youtube component from youtube url
dhaval-parekh Jun 24, 2021
d5f56f7
Add php unit test cases
dhaval-parekh Jun 24, 2021
080ded2
Convert youtube iframe into amp-youtube
dhaval-parekh Jun 29, 2021
a4e3290
Add additional data params for amp-youtube components
dhaval-parekh Jun 29, 2021
2e47705
Add unit test case for youtube embeds
dhaval-parekh Jun 29, 2021
55fff1f
Fix the issues from unit test cases
dhaval-parekh Jun 29, 2021
e5fba98
Fix the issues from unit test cases
dhaval-parekh Jun 29, 2021
92ab4be
Use type casting instead of intval()
dhaval-parekh Jun 30, 2021
e9ad832
Add inline comments, remove unwanted code
dhaval-parekh Jun 30, 2021
a5d73e0
Update data-param for amp-youtube
dhaval-parekh Jun 30, 2021
eceb487
Update XPath query to fetch youtube iframe
dhaval-parekh Jul 1, 2021
8804f83
Fix php unit test cases
dhaval-parekh Jul 1, 2021
220af89
Apply suggestions from PR
dhaval-parekh Jul 7, 2021
cae32c0
Update amp youtibe embed handler
dhaval-parekh Jul 7, 2021
12a722c
update unit test cases
dhaval-parekh Jul 7, 2021
520be01
Update and fix unit test cases
dhaval-parekh Jul 7, 2021
8e484f0
Remove redundant code
dhaval-parekh Jul 7, 2021
6808668
Add support of live channel when added as raw iframe
dhaval-parekh Jul 8, 2021
f60fd46
Fix unit test cases
dhaval-parekh Jul 8, 2021
c35ecd9
Add unit test cases for live streaming channel
dhaval-parekh Jul 8, 2021
547f237
Update prepare_attribute()
dhaval-parekh Jul 9, 2021
32e9b2d
Update regex for to find seek time from URL
dhaval-parekh Jul 9, 2021
2d1785a
Add additional unit test cases for video seek time
dhaval-parekh Jul 9, 2021
3ecad1e
Sanitize attribute name
dhaval-parekh Sep 9, 2021
74dfcb8
update unit test cases
dhaval-parekh Sep 9, 2021
7f21703
Revert phpstan ignores
westonruter Sep 9, 2021
2c3ed4d
Try adding phpstan version to composer cache key
westonruter Sep 9, 2021
d519220
Revert "Revert phpstan ignores"
westonruter Sep 9, 2021
0524ae5
Print phpstan version when running on GHA
westonruter Sep 9, 2021
b9827d7
Try removing phpstan before composer install
westonruter Sep 9, 2021
fa6ee15
Fix yml syntax error
westonruter Sep 9, 2021
f93b9a1
Try downloading a specific version of phpstan instead of latest
westonruter Sep 9, 2021
9d51ef4
Try installing phpstan via shivammathur/setup-php
westonruter Sep 9, 2021
9d8e977
Fix issues identified by phpstan
westonruter Sep 9, 2021
8200f3f
Give more explicit array phpdoc type
westonruter Sep 9, 2021
edbcc6f
Remove unreachable code
westonruter Sep 9, 2021
32f358a
Opt to pass-through iframe for unrecognized YouTube URL rather than c…
westonruter Sep 9, 2021
1907df3
Remove condition which cannot be reached since video ID is always pro…
westonruter Sep 9, 2021
aac0e40
Use coversDefaultClass
westonruter Sep 9, 2021
d983fa4
Prevent generating closing IMG tags
westonruter Sep 9, 2021
9eefef9
Let placeholder link point to YouTube permalink and not iframe src
westonruter Sep 9, 2021
46bc356
Fix adding start to placeholder link; improve coverage
westonruter Sep 9, 2021
da36d2d
Make placeholder dependency on video ID more explicit
westonruter Sep 9, 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
89 changes: 89 additions & 0 deletions includes/embeds/class-amp-youtube-embed-handler.php
Original file line number Diff line number Diff line change
@@ -5,6 +5,8 @@
* @package AMP
*/

use AmpProject\Dom\Document;

/**
* Class AMP_YouTube_Embed_Handler
*
@@ -77,6 +79,93 @@ public function unregister_embed() {
remove_filter( 'embed_oembed_html', [ $this, 'filter_embed_oembed_html' ], 10 );
}

/**
* Sanitize HTML that are not added via Gutenberg.
*
* @param Document $dom Document.
*
* @return void
*/
public function sanitize_raw_embeds( Document $dom ) {

$nodes = $dom->xpath->query( '//iframe[ contains( @src, "youtu" ) ]' );

foreach ( $nodes as $node ) {

$html = $dom->saveHTML( $node );
$url = $node->getAttribute( 'src' );
$amp_youtube_node = $this->process_embed( $dom, $html, $url );

if ( ! empty( $amp_youtube_node ) ) {
$node->parentNode->replaceChild( $amp_youtube_node, $node );
}
}

}

/**
* To AMP youtube component from DOM Document.
*
* @param Document $dom Document DOM.
* @param string $html HTML markup of youtube iframe.
* @param string $url Youtube URL.
*
* @return DOMElement|false DOMElement on success, Otherwise false.
*/
public function process_embed( Document $dom, $html, $url ) {

$id = $this->get_video_id_from_url( $url );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized the AMP component can also [accept a YouTube channel id](Youtube channel id), so we need to provide support for that as well. So if there is no video ID, then check for a live channel ID.

From the docs:

For example, in this URL: https://www.youtube.com/embed/live_stream?channel=UCB8Kb4pxYzsDsHxzBfnid4Q, UCB8Kb4pxYzsDsHxzBfnid4Q is the channel id. You can provide a data-live-channelid instead of a data-videoid attribute to embed a stable url for a live stream instead of a video.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pierlon I have added support for live streaming channels for iframe added as custom HTML markup.
However, WordPress by default does not provide support for embedding YouTube URLs (It will just render as text)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If WordPress never supported it I think it's safe to say we don't have to either.

@pierlon I have added support for live streaming channels for iframe added as custom HTML markup.

I tried this but it parsed an incorrect video ID. Eg:

<iframe src="https://www.youtube.com/embed/live_stream?channel=12345" allowfullscreen="" width="560" height="315"></iframe>

Gives:

<amp-youtube data-videoid="live_stream" layout="responsive" width="560" height="315" data-param-channel="12345" ...</amp-youtube>

Instead, the data-videoid attribute should not have been set, nor the data-param-channel attribute. Perhaps we could add a condition to say if the parsed video ID is live_stream then it's invalid and bail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have addressed this issue.


if ( ! $id ) {
return false;
}

$args = $this->parse_props( $html, $url, $id );
if ( empty( $args ) ) {
return false;
}

$args['video_id'] = $id;

$args = wp_parse_args(
$args,
[
'video_id' => false,
'layout' => 'responsive',
'width' => $this->args['width'],
'height' => $this->args['height'],
'placeholder' => '',
]
);

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

$this->did_convert_elements = true;

$attributes = array_merge(
[ 'data-videoid' => $args['video_id'] ],
wp_array_slice_assoc( $args, [ 'layout', 'width', 'height' ] )
);
if ( ! empty( $args['title'] ) ) {
$attributes['title'] = $args['title'];
}

return AMP_DOM_Utils::create_node(
$dom,
'amp-youtube',
$attributes
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget the placeholder element:

Suggested change
return AMP_DOM_Utils::create_node(
$dom,
'amp-youtube',
$attributes
);
$placeholder_node = AMP_DOM_Utils::create_node(
$dom,
Tag::A,
[
Attribute::PLACEHOLDER => '',
Attribute::HREF => esc_url_raw( $url ),
]
);
$amp_component = AMP_DOM_Utils::create_node(
$dom,
'amp-youtube',
$attributes
);
$amp_component->appendChild( $placeholder_node );
return $amp_component;

}

/**
* Filter oEmbed HTML for YouTube to convert to AMP.
*