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

Pinterest oEmbed fails with .co.uk TLD #7968

Closed
gemmagarner opened this issue Oct 11, 2017 · 5 comments
Closed

Pinterest oEmbed fails with .co.uk TLD #7968

gemmagarner opened this issue Oct 11, 2017 · 5 comments
Labels
[Feature] Shortcodes / Embeds Good For Community [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended

Comments

@gemmagarner
Copy link

Inserting a Pinterest URL containing a .co.uk TLD does not embed a board, profile or individual pins.

Steps to reproduce the issue

  1. Go to pinterest.co.uk
  2. Copy the URL of a board containing a Pinterest .co.uk TLD. For example:

https://www.pinterest.co.uk/stayandroam/blue-and-white/

  1. Go to WordPress.com
  2. Go to Blog Posts > Add New
  3. Click on the Visual editor tab
  4. Paste in the URL from step 2 (.co.uk TLD)
  5. Publish the post
  6. View the post on the front end

What I expected

I expected to see the Pinterest board embedded in my post.

What happened instead

The Pinterest board was displayed as a URL rather than embedded into the post. However, I was able to successfully embed the Pinterest board by changing the Pinterest TLD from .co.uk to .com:

https://www.pinterest.com/stayandroam/blue-and-white/

Once the URL had been updated and the post was saved, the Pinterest board was embedded.

Screenshot

pinterest-embed

Code

pinterest.com appears to be hard coded here:

wp_embed_register_handler( 'pinterest', '#https?://(?:www\.)?(?:[a-z]{2}\.)?pinterest\.com/([^/]+)(/[^/]+)?#', 'pinterest_embed_handler' );

Reported in 879741-f

@jeherve jeherve added [Feature] Shortcodes / Embeds [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended Good For Community and removed General labels Oct 11, 2017
@lizkarkoski
Copy link

There's an additional report of this for the post https://diywardrobe.co.uk/2017/11/07/curated-closet-part-2-inspiration/ which was reported in the forum thread at http://en.forums.wordpress.com/topic/cant-embed-pinterest-board

@gemmagarner
Copy link
Author

I've come across the same issue with .pt domains in 892882-f. I tested embedding with that extension and it did not work.

zinigor added a commit that referenced this issue Mar 16, 2018
Fixes #7968. For URLs other than pinterest.com that are embedded, the embed doesn't pick them up. This change modifies the regular expression to include any domain that has pinterest. followed with TLDs. This allows for false positives for garbage links and sites not affiliated with pinterest ( like http://pinterest.example.com/something )  to also work with this embed, but it shouldn't be a problem.
dereksmart pushed a commit that referenced this issue Mar 27, 2018
Fixes #7968. For URLs other than pinterest.com that are embedded, the embed doesn't pick them up. This change modifies the regular expression to include any domain that has pinterest. followed with TLDs. This allows for false positives for garbage links and sites not affiliated with pinterest ( like http://pinterest.example.com/something )  to also work with this embed, but it shouldn't be a problem.
@darcie
Copy link
Member

darcie commented Apr 11, 2018

I saw the topic on this in the forums and came here, as that topic was closed. If you've only added .co.uk, there will still be outstanding Pinterest users that come back about the inability to embed. Pinterest shifted from using uk.pinterest.com to www.pinterest.co.uk and other similar tlds a few months ago. So .co.uk works now to embed on WordPress.com in addition to pinterest.com, but .ca or .de does not.

As I understand, Pinterest is going to keep adding TLDs, and so there are still more to add to embeds and will likely be more in the future.

I understand that the latest commit was intended to allow more domains, but in my testing it does not, to clarify further. Testing: https://darciemg.wordpress.com/2018/01/04/changed-post-title/

@jeherve
Copy link
Member

jeherve commented Apr 12, 2018

@darcie We've added support for any other TLDs, so that should be okay. You won't see the fix on your blog just yet, though, as it hasn't been added to WordPress.com sites yet, only to Jetpack sites.

We'll do that soon though!

@darcie
Copy link
Member

darcie commented Apr 12, 2018

Thanks for the clarification, @jeherve! I can use them all successfully on a Jetpack site, and look forward to it rolling out to WordPress.com sites as well. I appreciate the followup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Shortcodes / Embeds Good For Community [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

No branches or pull requests

4 participants