Amp-wp: Pinterest embed handler URL pattern is incomplete

Created on 28 Aug 2020  ·  6Comments  ·  Source: ampproject/amp-wp

Bug Description

Many valid Pinterest pin URLs are not captured by the URL pattern within class-amp-pinterest-embed-handler.php.

Pinterest runs a large number of regional variants of their site. For example, these pins are identical, but hosted on different subdomains:

On top of that, some subdomains actually redirect to a different TLD, e.g.:

And there of course is the same pin hosted referenced from the regional TLD as well:

Expected Behaviour


All valid Pinterest pin URLs are captured and the content is embedded.

Steps to reproduce

  1. Create a new post and add a Custom HTML block.
  2. Insert a "regional" Pinterest URL, e. g. https://cz.pinterest.com/pin/8092474319950168/
  3. Save the post and view it.
  4. Notice the embed is not displayed. Instead, the URL is displayed verbatim.
  5. Edit the post and change the Custom HTML block contents to https://pinterest.com/pin/8092474319950168/.
  6. Save the post and view it.
  7. Notice the embed is properly displayed.

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

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

Bug Changelogged Embeds Core

All 6 comments

I'm surprised that WordPress core does not include Pinterest among its oEmbed providers.

We can include this fix in 2.0.1.

Hi @dero, can you please verify that the issue is fixed by testing the build linked here: https://github.com/ampproject/amp-wp/pull/5299#issuecomment-683334330.

Hi @pierlon – thank you, I have verified those changes address issues outlined in this ticket. 👍

However, I'm also going to follow up on the PR with a couple comments.

QA Passed

Testing with this content:

image

Before | After
-------|------
image | image

In testing I noticed the Pins do not render well. I've worked out an improved approach to rendering that ensures the correct height is applied to amp-pinterest by using Pinterest REST API call. This is proposed for the Jetpack Pinterest block, but we should port it into the AMP plugin: https://github.com/Automattic/jetpack/pull/17086

Aside: I wonder why Jetpack doesn't register an oEmbed handler so that the Embed block can't be used.

We should also implement support for converting raw Pinterest embeds.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

miina picture miina  ·  3Comments

swissspidy picture swissspidy  ·  5Comments

luizeof picture luizeof  ·  4Comments

westonruter picture westonruter  ·  5Comments

ernee picture ernee  ·  4Comments