Provide a placeholder element for Twitter embeds by westonruter · Pull Request #1753 · ampproject/amp-wp

westonruter · 2018-12-19T06:37:50Z

Refer to prior work on SoundCloud embeds (#2722) and YouTube embeds (#3358).

Todo:

Use Twitter oEmbed response as placeholder content for amp-twitter.
Update tests to mock HTTP requests to prevent external requests during testing.
Fix tests.
~~Use YouTube poster image linked to YouTube URL as the fallback content for amp-youtube.~~ See Provide a placeholder element for Twitter embeds #1753 (comment). Done in Improve handling of YouTube embeds #3358.

Twitter

Before:

<amp-twitter data-tweetid="1065026506068504577" layout="responsive" width="600" height="480"></amp-twitter>

After:

<amp-twitter width="600" height="480" layout="responsive" data-tweetid="1065026506068504577">
    <blockquote class="twitter-tweet" data-width="525" data-dnt="true" placeholder>
        <p lang="en" dir="ltr">🦖 HTM: JSX without the transpiler.<br><br>700 bytes to run in the browser,<br>or
            compile it away using babel-plugin-htm.<br><br>(works with any VDOM library!)<a href="https://t.co/KrlyiYP9Aj">https://t.co/KrlyiYP9Aj</a>
            <a href="https://t.co/qbEFfMT1Dy">pic.twitter.com/qbEFfMT1Dy</a></p>— Jason Miller 🦊⚛ (@_developit) <a
            href="https://twitter.com/_developit/status/1065026506068504577?ref_src=twsrc%5Etfw">November 20, 2018</a>
    </blockquote>
</amp-twitter>

westonruter · 2019-07-16T00:35:47Z

~~Actually, fallback should be used instead of placeholder.~~

Also, this also needs to be done for amp-youtube, though there is not the same amount of content available for use as fallback. For YouTube embedding https://www.youtube.com/watch?v=8EtmqU844hU on a non-AMP page results in the markup:

<figure class="wp-block-embed-youtube wp-block-embed is-type-video is-provider-youtube wp-embed-aspect-16-9 wp-has-aspect-ratio">
    <div class="wp-block-embed__wrapper">
        <iframe title="Finding Portland" width="640" height="360" src="https://www.youtube.com/embed/8EtmqU844hU?feature=oembed" frameborder="0" allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture" allowfullscreen></iframe>
    </div>
</figure>

And the AMP version is:

<figure class="wp-block-embed-youtube wp-block-embed is-type-video is-provider-youtube  wp-has-aspect-ratio">
    <div class="wp-block-embed__wrapper">
        <amp-youtube data-videoid="8EtmqU844hU" layout="responsive" width="640" height="360"></amp-youtube>
    </div>
</figure>

In actuality, the AMP version should have placeholder content that links to the original URL on YouTube, with the title of the iframe provided as the link text. In other words:

<figure class="wp-block-embed-youtube wp-block-embed is-type-video is-provider-youtube  wp-has-aspect-ratio">
   <div class="wp-block-embed__wrapper">
       <amp-youtube data-videoid="8EtmqU844hU" layout="responsive" width="640" height="360">
           <a fallback href="https://www.youtube.com/watch?v=8EtmqU844hU">Finding Portland</a>
       </amp-youtube>
   </div>
</figure>

This will ensure that the linked content is accessible to search engines and also for users who have JS turned off. This should be done generally for all embeds.

For YouTube in particular, a placeholder is also appropriate in that we can use the poster image as provided by YouTube.

<figure class="wp-block-embed-youtube wp-block-embed is-type-video is-provider-youtube  wp-has-aspect-ratio">
   <div class="wp-block-embed__wrapper">
       <amp-youtube data-videoid="8EtmqU844hU" layout="responsive" width="640" height="360">
           <a fallback href="https://www.youtube.com/watch?v=8EtmqU844hU">Finding Portland</a>
           <amp-img placeholder src="https://img.youtube.com/vi/8EtmqU844hU/maxresdefault.jpg" layout="fill"></amp-img>
       </amp-youtube>
   </div>
</figure>

…s class name counterpart

googlebot · 2019-11-15T03:09:44Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

pierlon · 2019-11-15T03:10:15Z

@googlebot I consent.

googlebot · 2019-11-15T03:10:23Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

pierlon · 2019-11-15T03:30:03Z

@westonruter should there also be a fallback element for amp-twitter?

westonruter · 2019-11-15T03:42:38Z

I don't think so because the placeholder has everything. We ended up not using fallback for YouTube either.

pierlon · 2019-11-15T03:47:31Z

Ah that's what I thought as well, but the PR title made me think different.

pierlon

No issues to report here,

westonruter · 2019-11-15T04:49:53Z

+		add_filter( 'embed_oembed_html', [ $this, 'filter_embed_oembed_html' ], 10, 2 );
 		add_shortcode( 'tweet', [ $this, 'shortcode' ] ); // Note: This is a Jetpack shortcode.
-		wp_embed_register_handler( 'amp-twitter', self::URL_PATTERN, [ $this, 'oembed' ], -1 );
 		wp_embed_register_handler( 'amp-twitter-timeline', self::URL_PATTERN_TIMELINE, [ $this, 'oembed_timeline' ], -1 );


Eventually the same thing should be done for Twitter timelines.

westonruter · 2019-11-15T04:50:09Z

 	 * @return string Rendered oEmbed.
 	 */
 	public function oembed( $matches ) {
+		_deprecated_function( __FUNCTION__, '1.1' );


Suggested change

_deprecated_function( __FUNCTION__, '1.1' );

_deprecated_function( __METHOD__, '1.1' );

csossi · 2019-11-19T15:15:10Z

Verified in QA

westonruter added this to the v1.1 milestone Dec 19, 2018

googlebot added the cla: yes Signed the Google CLA label Dec 19, 2018

westonruter force-pushed the add/twitter-embed-placeholder branch from d84c7b0 to 90861ae Compare December 19, 2018 06:48

westonruter removed this from the v1.1 milestone Mar 6, 2019

westonruter self-assigned this Jul 16, 2019

westonruter added the Embeds label Aug 20, 2019

westonruter added this to the v1.2.2 milestone Aug 20, 2019

westonruter mentioned this pull request Aug 20, 2019

amp-twitter sanitiser doesn't respect data-lang or data-conversation attributes #3050

Closed

westonruter modified the milestones: v1.2.2, v1.3 Aug 27, 2019

westonruter modified the milestones: v1.3, v1.3.1 Sep 18, 2019

swissspidy modified the milestones: v1.3.1, v1.4 Oct 17, 2019

westonruter removed this from the v1.4 milestone Oct 17, 2019

westonruter force-pushed the add/twitter-embed-placeholder branch from 90861ae to 4ce50e9 Compare October 24, 2019 17:34

westonruter removed their assignment Oct 24, 2019

westonruter changed the title ~~[WIP] Use normal Twitter oEmbed response as amp-twitter placeholder~~ [WIP] Provide placeholder and fallback content for Twitter and YouTube embeds Oct 24, 2019

westonruter changed the title ~~[WIP] Provide placeholder and fallback content for Twitter and YouTube embeds~~ [WIP] Provide placeholder and fallback content for Twitter embeds Nov 12, 2019

westonruter force-pushed the add/twitter-embed-placeholder branch from 4ce50e9 to 18f39dc Compare November 12, 2019 17:37

jauyong added the Size: XS label Nov 12, 2019

westonruter and others added 5 commits November 14, 2019 19:23

Use normal Twitter oEmbed response as amp-twitter placeholder

7a9f87f

Rename the file for the AMP_Twitter_Embed_Handler class to match it…

1ad19e3

…s class name counterpart

Line feeds (\n) may be included in the oembed response

020b37a

Mock HTTP requests

d827655

Fix failing tests

a9ed369

pierlon force-pushed the add/twitter-embed-placeholder branch from 18f39dc to a9ed369 Compare November 15, 2019 03:09

googlebot removed the cla: yes Signed the Google CLA label Nov 15, 2019

googlebot added the cla: no Has not signed the Google CLA label Nov 15, 2019

googlebot added cla: yes Signed the Google CLA and removed cla: no Has not signed the Google CLA labels Nov 15, 2019

pierlon changed the title ~~[WIP] Provide placeholder and fallback content for Twitter embeds~~ Provide placeholder and fallback content for Twitter embeds Nov 15, 2019

pierlon self-assigned this Nov 15, 2019

pierlon changed the title ~~Provide placeholder and fallback content for Twitter embeds~~ Provide a placeholder element for Twitter embeds Nov 15, 2019

pierlon approved these changes Nov 15, 2019

View reviewed changes

Use __METHOD__ in _deprecated_function(); update comments

567f3ed

westonruter commented Nov 15, 2019

View reviewed changes

westonruter added this to the v1.5 milestone Nov 15, 2019

westonruter merged commit 5c3d911 into develop Nov 15, 2019

westonruter deleted the add/twitter-embed-placeholder branch November 15, 2019 05:53

westonruter mentioned this pull request Dec 4, 2019

Prevent wpautop() from modifying Twitter embed #3874

Merged

3 tasks

csossi added the QA passed Has passed QA and is done label Feb 6, 2020

westonruter added the Changelogged label Mar 22, 2020

Navigation Menu

Uh oh!

Provide a placeholder element for Twitter embeds - #1753

Provide a placeholder element for Twitter embeds#1753
westonruter merged 6 commits into
developfrom
add/twitter-embed-placeholder

westonruter commented Dec 19, 2018 •

edited

Loading

Uh oh!

westonruter commented Jul 16, 2019 •

edited

Loading

Uh oh!

googlebot commented Nov 15, 2019

Uh oh!

pierlon commented Nov 15, 2019

Uh oh!

googlebot commented Nov 15, 2019

Uh oh!

pierlon commented Nov 15, 2019

Uh oh!

westonruter commented Nov 15, 2019

Uh oh!

pierlon commented Nov 15, 2019

Uh oh!

pierlon left a comment

Uh oh!

westonruter Nov 15, 2019

Uh oh!

westonruter Nov 15, 2019

Uh oh!

csossi commented Nov 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	_deprecated_function( __FUNCTION__, '1.1' );
	_deprecated_function( __METHOD__, '1.1' );

Uh oh!

Conversation

westonruter commented Dec 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Twitter

Uh oh!

westonruter commented Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Nov 15, 2019

Uh oh!

pierlon commented Nov 15, 2019

Uh oh!

googlebot commented Nov 15, 2019

Uh oh!

pierlon commented Nov 15, 2019

Uh oh!

westonruter commented Nov 15, 2019

Uh oh!

pierlon commented Nov 15, 2019

Uh oh!

pierlon left a comment

Choose a reason for hiding this comment

Uh oh!

westonruter Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

westonruter Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

csossi commented Nov 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

westonruter commented Dec 19, 2018 •

edited

Loading

westonruter commented Jul 16, 2019 •

edited

Loading