Amp-wp: Twitter raw embeds are not all converted

[westonruter]

Bug Description

There are various options for embedding content from Twitter, as can be seen at https://publish.twitter.com/:

Of all these options, only the Tweet currently is properly converted.

This necessitated Jetpack adding special support for embedding Twitter timelines: https://github.com/Automattic/jetpack/pull/15328

This should not have been the case, however: https://github.com/Automattic/jetpack/pull/15328/files#r419632716

Expected Behaviour

Raw embeds code coming from Twitter should be converted into AMP components as required.

Steps to reproduce

Paste this markup while editing code in the block editor:

<!-- wp:heading -->
<h2>Collection</h2>
<!-- /wp:heading -->

<!-- wp:html -->
<a class="twitter-timeline" href="https://twitter.com/TwitterDev/timelines/539487832448843776?ref_src=twsrc%5Etfw">National Park Tweets - Curated tweets by TwitterDev</a> <script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
<!-- /wp:html -->

<!-- wp:heading -->
<h2>Tweet</h2>
<!-- /wp:heading -->

<!-- wp:html -->
<blockquote class="twitter-tweet"><p lang="en" dir="ltr">Sunsets don&#39;t get much better than this one over <a href="https://twitter.com/GrandTetonNPS?ref_src=twsrc%5Etfw">@GrandTetonNPS</a>. <a href="https://twitter.com/hashtag/nature?src=hash&amp;ref_src=twsrc%5Etfw">#nature</a> <a href="https://twitter.com/hashtag/sunset?src=hash&amp;ref_src=twsrc%5Etfw">#sunset</a> <a href="http://t.co/YuKy2rcjyU">pic.twitter.com/YuKy2rcjyU</a></p>&mdash; US Department of the Interior (@Interior) <a href="https://twitter.com/Interior/status/463440424141459456?ref_src=twsrc%5Etfw">May 5, 2014</a></blockquote> <script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
<!-- /wp:html -->

<!-- wp:heading -->
<h2>Profile: Embedded Timeline</h2>
<!-- /wp:heading -->

<!-- wp:html -->
<a class="twitter-timeline" href="https://twitter.com/TwitterDev?ref_src=twsrc%5Etfw">Tweets by TwitterDev</a> <script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
<!-- /wp:html -->

<!-- wp:heading -->
<h2>Profile: Follow Button</h2>
<!-- /wp:heading -->

<!-- wp:html -->
<a href="https://twitter.com/TwitterDev?ref_src=twsrc%5Etfw" class="twitter-follow-button" data-show-count="false">Follow @TwitterDev</a><script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
<!-- /wp:html -->

<!-- wp:heading -->
<h2>Profile: Mention Button</h2>
<!-- /wp:heading -->

<!-- wp:html -->
<a href="https://twitter.com/intent/tweet?screen_name=TwitterDev&ref_src=twsrc%5Etfw" class="twitter-mention-button" data-show-count="false">Tweet to @TwitterDev</a><script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
<!-- /wp:html -->

<!-- wp:heading -->
<h2>List</h2>
<!-- /wp:heading -->

<!-- wp:html -->
<a class="twitter-timeline" href="https://twitter.com/TwitterDev/lists/national-parks?ref_src=twsrc%5Etfw">A Twitter List by TwitterDev</a> <script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
<!-- /wp:html -->

<!-- wp:heading -->
<h2>Moment</h2>
<!-- /wp:heading -->

<!-- wp:html -->
<a class="twitter-moment" href="https://twitter.com/i/moments/625792726546558977?ref_src=twsrc%5Etfw">Michelle Obama Opens Special Olympics</a> <script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
<!-- /wp:html -->

<!-- wp:heading -->
<h2>Likes Timeline</h2>
<!-- /wp:heading -->

<!-- wp:html -->
<a class="twitter-timeline" href="https://twitter.com/TwitterDev/likes?ref_src=twsrc%5Etfw">Tweets Liked by @TwitterDev</a> <script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
<!-- /wp:html -->

<!-- wp:heading -->
<h2>Hashtag Button</h2>
<!-- /wp:heading -->

<!-- wp:html -->
<a href="https://twitter.com/intent/tweet?button_hashtag=LoveTwitter&ref_src=twsrc%5Etfw" class="twitter-hashtag-button" data-show-count="false">Tweet #LoveTwitter</a><script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
<!-- /wp:html -->

Screenshots

Validation errors are raised for all embeds other than Tweets:

---

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

Implementation brief

The logic in AMP_Twitter_Embed_Handler::is_tweet_raw_embed() is lacking:

https://github.com/ampproject/amp-wp/blob/4880f0f58daaf07685854be8574ff25d76ff583e/includes/embeds/class-amp-twitter-embed-handler.php#L132-L147

Namely, in addition to checking for twitter-tweet it also needs to account for all the other class names that the embeds utilize, including twitter-timeline, twitter-hashtag-button, twitter-moment and so on.

QA testing instructions

Demo

Changelog entry

[pierlon]

This may be unintentionally fixed by #4650 as I noticed some of the tests in AMP_Twitter_Embed_Handler_Test began to fail due to them not being sanitized properly by AMP_Twitter_Embed_Handler::sanitize_raw_embeds. I'll ensure this issue is accounted for in the aforementioned PR.

[pierlon]

The "follow", "hashtag", and "mention" buttons do not seem to be supported by the amp-twitter component.

When the embed is rendered by the accompanied script it produces an iframe:

<iframe id="twitter-widget-0" scrolling="no" frameborder="0" allowtransparency="true" allowfullscreen="true" class="twitter-follow-button twitter-follow-button-rendered" style="position: static; visibility: visible; width: 129px; height: 20px;" title="Twitter Follow Button" src="https://platform.twitter.com/widgets/follow_button.c63890edc4243ee77048d507b181eeec.en.html#dnt=true&amp;id=twitter-widget-0&amp;lang=en&amp;screen_name=TwitterDev&amp;show_count=false&amp;show_screen_name=true&amp;size=m&amp;time=1588909307584" data-screen-name="TwitterDev"></iframe>

We could attempt to create an amp-iframe and imitate the iframe src URL above, but that seems like a brittle solution to the problem. Any suggestions?

[westonruter]

It looks like there is a feature request to support this: https://github.com/ampproject/amphtml/issues/24678

In the meantime, we'll have to just skip converting such elements. At least the link is a suitable fallback.