Make WordPress Core

Opened 8 years ago

Closed 7 years ago

Last modified 6 years ago

#38181 closed enhancement (fixed)

Add oEmbed support for Amazon Kindle Instant Previews

Reported by: jsepia's profile jsepia Owned by: johnbillion's profile johnbillion
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.7
Component: Embeds Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Kindle Instant Previews is a service that allows authors, media outlets and bloggers to embed a fully functional preview of a Kindle book into their site.

On September 10th, we launched our oEmbed endpoint. This patch allows Wordpress users to embed book previews from existing Kindle Instant Previews URLs as well as amazon.com URLs.

Attachments (9)

38181.diff (4.1 KB) - added by jsepia 8 years ago.
Changes to class-oembed.php to support Kindle embeds
38181.2.diff (4.6 KB) - added by morganestes 8 years ago.
Updated patch to fix some standards issues.
38181.3.diff (5.6 KB) - added by adamsilverstein 8 years ago.
38181.4.diff (12.0 KB) - added by jsepia 8 years ago.
Removed TLD routing filter, fixed the unit tests and added new ones
38181.5.diff (23.5 KB) - added by swissspidy 8 years ago.
38181.6.diff (8.5 KB) - added by jbpaul17 7 years ago.
Refreshed patch to build off of #38367
38181.7.diff (8.4 KB) - added by jbpaul17 7 years ago.
Refreshed for 4.9
38181.8.diff (3.0 KB) - added by johnbillion 7 years ago.
38181.9.diff (3.0 KB) - added by johnbillion 7 years ago.

Download all attachments as: .zip

Change History (61)

@jsepia
8 years ago

Changes to class-oembed.php to support Kindle embeds

#1 @jsepia
8 years ago

One caveat:

The regex we used to filter out Amazon URLs (OEMBED_AMAZON_FORMAT ) is meant to catch both Kindle preview URLs (example) as well as Amazon.com store URLs (example).

However, our API only handles Kindle URLs at the moment - handling store URLs is scheduled for an upcoming release.

Nevertheless, we would like Wordpress to handle both URL patterns from the get go. That way, all we have to do to enable support for store URLs is turn on that capability on our end.

Does everyone agree on this course of action?

This ticket was mentioned in Slack in #core by jsepia. View the logs.


8 years ago

#3 @jsepia
8 years ago

  • Keywords has-patch added

#4 @morganestes
8 years ago

Based on some discussion in Core chat, let's look at either 1) extracting this into a plugin, or 2) reworking it to match WP coding style a bit more by remove the constant, working on the style for the switch, and adding tests.

#5 follow-up: @jsepia
8 years ago

@morganestes Thanks for your feedback!

1) extracting this into a plugin

To clarify, our goal is to be whitelisted as a default oEmbed provider just like SlideShare, Vimeo, etc. Distributing our oEmbed handler as a plugin is not an option for us.

2) reworking it to match WP coding style a bit more by remove the constant, working on the style for the switch, and adding tests.

Sounds good.

#6 in reply to: ↑ 5 @morganestes
8 years ago

Replying to jsepia:

I'm with you that the ultimate goal is to be whitelisted, so let's work on that first. If for some reason it doesn't get committed, you can easily fall back to a plugin, but let's shoot for core.

#7 @iamfriendly
8 years ago

Just as a very minor note to save churn when you update your patch, your docblock for _match_amazon_tld() says

@since 4.6.2

But I think this would be a candidate for version 4.7.0.

Last edited 8 years ago by iamfriendly (previous) (diff)

#8 @morganestes
8 years ago

  • Keywords needs-testing needs-unit-tests added
  • Milestone changed from Awaiting Review to 4.7
  • Type changed from defect (bug) to enhancement

In 38181.2.diff:

  • updated to branch off of [38367]
  • moved the main URL RegEx inside the class as a static variable
  • updated formatting for the callback switch statement
  • added docs to the filter to add Amazon Kindle
  • changed @since version to 4.7.0

Tested locally with https://read.amazon.com/kp/embed?asin=B00RZDEKVA&preview=newtab&linkCode=kpe&ref_=cm_sw_r_kb_dp_YPg7xbGKSPV24, but needs unit tests for full range of URL possibilities.

@morganestes
8 years ago

Updated patch to fix some standards issues.

#9 follow-ups: @ocean90
8 years ago

@jsepia Could you please answer the questions on whitelisting providers? This will help us in the decision process.

#10 in reply to: ↑ 9 ; follow-up: @jsepia
8 years ago

Replying to morganestes:

In 38181.2.diff:

  • updated to branch off of [38367]
  • moved the main URL RegEx inside the class as a static variable
  • updated formatting for the callback switch statement
  • added docs to the filter to add Amazon Kindle
  • changed @since version to 4.7.0

Tested locally with https://read.amazon.com/kp/embed?asin=B00RZDEKVA&preview=newtab&linkCode=kpe&ref_=cm_sw_r_kb_dp_YPg7xbGKSPV24, but needs unit tests for full range of URL possibilities.

Thanks for cleaning up the patch!

Replying to ocean90:

@jsepia Could you please answer the questions on whitelisting providers? This will help us in the decision process.

Sure. I'll have to consult my team leads on some of the questions. In the meantime, could you clarify:

Is its oEmbed endpoint [...] properly documented?

What kind of documentation does this question refer to? Can you provide some examples? I've read through a couple of past oEmbed whitelisting tickets but I was unable to find documentation on any of the providers.

#11 in reply to: ↑ 10 @swissspidy
8 years ago

Replying to jsepia:

Sure. I'll have to consult my team leads on some of the questions. In the meantime, could you clarify:

Is its oEmbed endpoint [...] properly documented?

What kind of documentation does this question refer to? Can you provide some examples? I've read through a couple of past oEmbed whitelisting tickets but I was unable to find documentation on any of the providers.

This is mainly about official documentation for developers. Sometimes a service kinda supports oEmbed, but it's nowhere announced. Here are some examples:

#12 @adamsilverstein
8 years ago

In 38181.3.diff:

  • refresh against trunk
  • add endpoint example to external-oembed group unit test oEmbedProviderData
  • restore remapping applied via oembed_fetch_url which went missing in the latest patch

here is a screenshot of the editor after pasting in the test URL:

https://cl.ly/0A0O1h3F0g2A/Add_New_Page__WordPress_Dev__WordPress_2016-10-05_16-30-15.jpg

front end:

https://cl.ly/1T1g112b2P1n/Hello_World__WordPress_Dev_2016-10-05_16-32-13.jpg

running the external-oembed phpunit tests I get some failures, is that expected? Adding this to the tests did change the number of failures.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


8 years ago

#14 follow-up: @dd32
8 years ago

Looking at the logic required for Amazon, I don't think it really makes sense for Core to add the extra routing support.

However, I think this is a prime case where the provider (Amazon) should enable oEmbed Discovery on the URLs so as to be able to handle the slightly complex routing on their side. As the Amazon oEmbed API returns an iframe, it should just work with recent versions of WordPress then (Assuming all the short url domains listed don't just redirect to a more full url).

@jsepia
8 years ago

Removed TLD routing filter, fixed the unit tests and added new ones

#15 @jsepia
8 years ago

We understand the concerns around having to support multiple domains. Our primary reason for doing it this way was making sure that customers' requests are always directed to the nearest, fastest servers.

I was able to find a solution with my team that allowed us to remove the troublesome TLD matching logic. In order to make sure that WordPress users don't experience overly high latency when embedding Kindle previews, we would like to maintain four different versions of the regex, each of one pointing to the dedicated endpoint for that region.

Replying to dd32:

Looking at the logic required for Amazon, I don't think it really makes sense for Core to add the extra routing support.

However, I think this is a prime case where the provider (Amazon) should enable oEmbed Discovery on the URLs so as to be able to handle the slightly complex routing on their side.

Agreed. That is definitely in the roadmap for us.

Last edited 8 years ago by jsepia (previous) (diff)

#16 in reply to: ↑ 14 @swissspidy
8 years ago

Replying to dd32:

Looking at the logic required for Amazon, I don't think it really makes sense for Core to add the extra routing support.

However, I think this is a prime case where the provider (Amazon) should enable oEmbed Discovery on the URLs so as to be able to handle the slightly complex routing on their side. As the Amazon oEmbed API returns an iframe, it should just work with recent versions of WordPress then (Assuming all the short url domains listed don't just redirect to a more full url).

oEmbed discovery doesn't quite work as the resulting <iframe> printed by WordPress is too restrictive for Amazon's content.

#17 @swissspidy
8 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Latest patch looks much better. Thanks @jsepia!

#18 in reply to: ↑ 9 @jsepia
8 years ago

Replying to ocean90:

@jsepia Could you please answer the questions on whitelisting providers? This will help us in the decision process.

You will find answers to all the inclusion criteria below.


Is the service is popular enough for core developers to have heard of it before? Is it “mainstream?”

We are in early stages of development for Kindle instant preview. We are currently focused on learning more about the right experience for end users and iterating quickly. We expect Kindle instant preview to be the go-to way people use to connect or reference a book just like people use YouTube to connect or reference a video.

If similar services are already supported, how does this service compare in terms of size, features, and backing?

Our service is free and accessible to any user, at any time – no sign-ups required. It is also a web-based reading experience so there is no app download required. Scribd does have a product that allows users add book previews to their site; however, users must be a Scribd member.

Does this service have a large following on Twitter, Facebook, or other social media?

Kindle instant preview does not have an active social media following (e.g. Mentions or dedicated pages); it is mentioned and used by the overall Kindle brand, which does have a social media presence across mediums. People use Kindle instant preview to talk about books on social media, not the product itself. For example, people use Vimeo to talk about a video on Facebook, but they rarely talk about Vimeo. The same would apply here.

Is its Twitter account verified? Is its oEmbed endpoint clearly established and properly documented? (Sometimes, they are just a developer’s pet project that may not be supported.)

This oEmbed endpoint is an official feature of Kindle Instant Previews. It has been rigorously tested within Amazon, and it is already in use by WordPress.com. We have not released any public documentation as of yet, but we do have plans to make our oEmbed feature known to the public.

Does the service make an effort to build relationships with developers, such as through robust APIs?

We have a few tools available to use it. We have a Product Advertising API solution as well as a self service tool available on amazon.com. Customers reference the following link to learn more about the self service tool: www.amazon.com/kindleinstantpreview

How old is the service?

This product is less than a year old.

Does it have a well-established Wikipedia article? (Seriously.)

Not currently. Customers reference the following link to learn more about the product, use case, and how to use it: www.amazon.com/kindleinstantpreview

Has anyone written a WordPress plugin that leverages the service in some way, whether adding it as an oEmbed provider, creating a shortcode, or leveraging other APIs of the service? Do these plugins have any noticeable adoption or traction that would indicate usage and demand?

We have noticed that the majority of our users are actually WordPress users who are using PAAPI to create previews on their site. We believe this indicates the popularity of this product among WordPress users. For example, many of our customers are authors or mainstream publishers who use the WordPress platform to discuss books.

Is the provider frequently proposed?

No, this is the first time.

Last edited 8 years ago by jsepia (previous) (diff)

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


8 years ago

This ticket was mentioned in Slack in #core by jsepia. View the logs.


8 years ago

#21 @jorbin
8 years ago

  • Owner set to swissspidy
  • Status changed from new to assigned

Are the embeds primarily aimed at selling books, or providing content for end users? My first reaction is that this seems to be a glorified advertising embed.

Also, Is this for embedding any amazon page? The regex picks up all amazon urls. What does the kindle instant preview look like if I use the URL for a pair of pants?

@swissspidy assigning to you as the component maintainer.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


8 years ago

#23 follow-up: @jsepia
8 years ago

Replying to jorbin:

Are the embeds primarily aimed at selling books, or providing content for end users? My first reaction is that this seems to be a glorified advertising embed.


Our primary goal is to allow people to explore books. We built Kindle instant preview to serve as a way for people to connect over books just like they can connect over video with Vimeo and YouTube and music with AudioBoom and SoundCloud.

  • End users can use KIP to get additional context on an article or page that mentions a book (example).
  • Authors can use KIP to share their own books (see example) so readers can get a sense of the writing style and storyline, which is a valuable web tool for authors.

Kip does include an option to purchase the book; however we designed Kip specifically to focus on bringing customers into a reading experience for a book they just discovered. We purposefully designed it to not look like an Amazon purchase experience by quietening the branding for Amazon and Kindle within the experience, ensuring that the content takes center stage. We appreciate your feedback and are sensitive to concerns around this product potentially being perceived in an ad; we are working to further improve the look and feel to address this soon.

Also, Is this for embedding any amazon page? The regex picks up all amazon urls. What does the kindle instant preview look like if I use the URL for a pair of pants?


For now, the service only provides embeds for KIP URLs (read.amazon.com et al).

We would like to generate embeds for any Amazon URL that represents a Kindle book (example), but whether we enable this feature will depend on multiple factors. For one, we need to verify and ensure that our service can handle the addidional load with no impact to the end user experience.

We currently have no plans to generate embeds for Amazon products that aren't books. Those will return a 404 as prescribed by the oEmbed spec.

#24 in reply to: ↑ 23 @swissspidy
8 years ago

Thanks for your answers, @jsepia!

For now, the service only provides embeds for KIP URLs (read.amazon.com et al).

We would like to generate embeds for any Amazon URL that represents a Kindle book (example), but whether we enable this feature will depend on multiple factors. For one, we need to verify and ensure that our service can handle the additional load with no impact to the end user experience.

We currently have no plans to generate embeds for Amazon products that aren't books. Those will return a 404 as prescribed by the oEmbed spec.

So right now, when I want to embed a book in a blog post, I cannot simply copy the URL in my browser's address bar and paste it into the editor. Instead, I need to follow the instructions on https://www.amazon.com/kindleinstantpreview, i.e. click the "Embed" link next to the other sharing options and copy the embed URL. Without the patch, the only difference is that I need to copy the embed code (<iframe…) instead. At least that's my understanding and if that is correct, there's no substantial benefit in implementing this.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


8 years ago

#26 @jbpaul17
8 years ago

  • Milestone changed from 4.7 to Future Release

#27 @jorbin
8 years ago

This was moved out of the milestone since we are closing in on the deadline for 4.7 enhancements. I would love to see this in plugin form to prove that it's something users are interested in as well as the ability to copy urls directly from the address bar and use them.

I further would really love if the url regex could be tightened up to only detect perhaps a subdirectory on the amazon site rather than any Amazon URL if the purpose is just to work with Kindle Books.

#28 follow-up: @jsepia
8 years ago

We recently added OEmbed support for Amazon URLs. You can embed a sample of almost any Kindle book by copying the book's URL from the Amazon.com detail page (or Amazon.co.uk, Amazon.fr, etc) and pasting it into a blog post. We've been testing this on WordPress.com for about a week now and we're seeing quite a bit of usage.

Regarding non-book products (e.g. a toaster), we haven't converged on what the right embed experience is for them, or if they should even be embeddable to begin with. For the time being, we believe it's better for our users if we just leave the URL untouched.

I hope this change addresses @swissspidy and @jorbin's concerns regarding the usefulness of our oEmbed provider. We will continue to improve our service to support more use cases and book formats in the future.

#29 in reply to: ↑ 28 ; follow-up: @jbpaul17
8 years ago

Replying to jsepia:

We recently added OEmbed support for Amazon URLs. You can embed a sample of almost any Kindle book by copying the book's URL from the Amazon.com detail page (or Amazon.co.uk, Amazon.fr, etc) and pasting it into a blog post. We've been testing this on WordPress.com for about a week now and we're seeing quite a bit of usage.

Regarding non-book products (e.g. a toaster), we haven't converged on what the right embed experience is for them, or if they should even be embeddable to begin with. For the time being, we believe it's better for our users if we just leave the URL untouched.

I hope this change addresses @swissspidy and @jorbin's concerns regarding the usefulness of our oEmbed provider. We will continue to improve our service to support more use cases and book formats in the future.

So is the current request to whitelist the Amazon OEmbed so that almost any Kindle book will embed the live preview of the book, but no non-book products will embed from the URL in a consumer's browser address bar?

#30 in reply to: ↑ 29 @rugved
8 years ago

Replying to jbpaul17:

Replying to jsepia:

We recently added OEmbed support for Amazon URLs. You can embed a sample of almost any Kindle book by copying the book's URL from the Amazon.com detail page (or Amazon.co.uk, Amazon.fr, etc) and pasting it into a blog post. We've been testing this on WordPress.com for about a week now and we're seeing quite a bit of usage.

Regarding non-book products (e.g. a toaster), we haven't converged on what the right embed experience is for them, or if they should even be embeddable to begin with. For the time being, we believe it's better for our users if we just leave the URL untouched.

I hope this change addresses @swissspidy and @jorbin's concerns regarding the usefulness of our oEmbed provider. We will continue to improve our service to support more use cases and book formats in the future.

So is the current request to whitelist the Amazon OEmbed so that almost any Kindle book will embed the live preview of the book, but no non-book products will embed from the URL in a consumer's browser address bar?

Hello jbpaul17,
Let me introduce myself, I'm Rugved and I'm teammate of jsepia. I'm taking over this WordPress related work from him as he has moved to another team. I'll be driving this work henceforth.

As for your question, yes, once Amazon OEmbed is whitelisted then almost any book(not just Kindle book as we have Kindle previews for many print books as well) will embed the live preview of the book when a user pastes the Amazon URL from his browser's address bar into his WordPress blog. And non-book products will not embed anything and will keep the URL as it is.

Please let me know if you have any questions. We are really looking forward to get this functionality in 4.8 release. We are sure this will delight WordPress customers once they see a live preview of their books using their Amazon URL.

Thanks,
Rugved

@swissspidy
8 years ago

#31 @swissspidy
8 years ago

38181.5.diff updates the patch against trunk. As some tests from #32360 have been removed, I left out the relevant tests here as well. Also, it seems like Amazon now returns embeds with different URLs, so I updated the assertions in test_amazon_kindle_preview_non_us_embed() accordingly.

@jbpaul17
7 years ago

Refreshed patch to build off of #38367

#32 follow-up: @jbpaul17
7 years ago

  • Milestone changed from Future Release to 4.8

38181.6.diff updates to branch off of changes from 40574.

#33 in reply to: ↑ 32 @rugved
7 years ago

Replying to jbpaul17:

38181.6.diff updates to branch off of changes from 40574.

Thanks for fixing the tests and marking it for 4.8 release! We appreciate your support here!

Is there a target release date for 4.8 release?

Thanks,
Rugved

#34 @swissspidy
7 years ago

@rugved It has been announced last week on make/core that the target date for the release of WordPress 4.8 is June 8. See https://make.wordpress.org/core/4-8/ for the schedule.

#35 @swissspidy
7 years ago

@johnbillion What's your opinion on the latest patch?

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


7 years ago

#37 follow-up: @ocean90
7 years ago

  • Milestone changed from 4.8 to Future Release

Moving to future because beta 1 has been released which means no more commits for any new enhancements or feature requests in this release cycle.

#38 in reply to: ↑ 37 @rugved
7 years ago

Replying to ocean90:

Moving to future because beta 1 has been released which means no more commits for any new enhancements or feature requests in this release cycle.

This was tagged for 4.8 release. May I know the reason why it is being pushed out to a future release? I thought it was ready to go based on the earlier comments by @swissspidy .

#39 @johnbillion
7 years ago

  • Milestone changed from Future Release to 4.9

This generally looks good.

A few points:

  • amzn.onl doesn't appear to exist or resolve.
  • The canonical TLD: comments in test_amazon_kindle_preview_non_us_embed() have gotten a bit muddled. They don't match the assertions. The tests pass, though.

I'd love to get complete test coverage for all our oEmbed providers, but network problems cause too many false failures. See #32360.

@jbpaul17
7 years ago

Refreshed for 4.9

#40 @jbpaul17
7 years ago

Aloha from WordCamp for Publishers contributor day! I updated the patch for 4.9, removed the non-functioning amz.onl / amzn.onl references, and tried to clean up the comments per @johnbillion's latest review.

#41 @johnbillion
7 years ago

  • Owner changed from swissspidy to johnbillion
  • Status changed from assigned to reviewing

#42 @johnbillion
7 years ago

  • Keywords needs-testing removed

I'd really like to get this in, but I'm not a fan of allowing embeds from any subdomain of the amazon domains (the ([a-z0-9-]+\.)?amazon\. portion of the regex).

@rugved Can this be restricted to ([read|www]\.)?amazon\. to only allow embeds from the read. and www. subdomains? See comment 23 for reference.

@johnbillion
7 years ago

#43 follow-up: @johnbillion
7 years ago

38181.8.diff is the format I'd like to use for this patch. Just need to confirm about switching to a specific list of allowed subdomains.

@johnbillion
7 years ago

#44 @johnbillion
7 years ago

  • Keywords commit added

#45 in reply to: ↑ 43 ; follow-up: @rugved
7 years ago

Replying to johnbillion:

38181.8.diff is the format I'd like to use for this patch. Just need to confirm about switching to a specific list of allowed subdomains.

Sorry for the delayed response on this. Only "read|www" would not work for us since we have different subdomains per marketplace. Hence we had the regex. But if you feel strongly about having specific subdomains, then this is the list of all subdomains we need to define in there:
www, read, lesen, lire, leggi, leer, ler

Please include these and let me know if you have any questions or concerns. Thanks!

#46 in reply to: ↑ 45 @jsepia
7 years ago

Replying to rugved:

Replying to johnbillion:

38181.8.diff is the format I'd like to use for this patch. Just need to confirm about switching to a specific list of allowed subdomains.

Sorry for the delayed response on this. Only "read|www" would not work for us since we have different subdomains per marketplace. Hence we had the regex. But if you feel strongly about having specific subdomains, then this is the list of all subdomains we need to define in there:
www, read, lesen, lire, leggi, leer, ler

Please include these and let me know if you have any questions or concerns. Thanks!


To add to Rugved's comment:

We also need to support the smile. subdomain. The correct regexes for all supported subdomain/TLD combinations are:

'#https?://((read|leer|ler|smile|www)\.)?amazon\.(com|com\.mx|com\.br|ca)/.*#i'
'#https?://((read|lesen|lire|leggi|leer|smile|www)\.)?amazon\.(co\.uk|de|fr|it|es|in|nl|ru)/.*#i'
'#https?://((read|smile|www)\.)?amazon\.(co\.jp|com\.au)/.*#i'
'#https?://((read|smile|www)\.)?amazon\.cn/.*#i'


Here are some valid test cases:

http://amazon.com/dp/B000SEGUDE
https://smile.amazon.com/dp/B00DPM7TIG
https://smile.amazon.co.uk/dp/B00DPM7TIG
https://smile.amazon.co.uk/dp/1476746583
https://ler.amazon.br/?asin=B00DPM7TIG&kcrFree=only
https://ler.amazon.br/kp/embed?asin=B00DPM7TIG
https://lesen.amazon.de/?asin=B00DPM7TIG&kcrFree=only
https://lesen.amazon.de/kp/embed?asin=B00DPM7TIG
https://read.amazon.cn/?asin=B00DPM7TIG&kcrFree=only
https://read.amazon.cn/kp/embed?asin=B00DPM7TIG
Last edited 7 years ago by jsepia (previous) (diff)

#47 @jbpaul17
7 years ago

  • Keywords needs-refresh added

Refresh needed to update the regex for relevant subdomains.

#48 @johnbillion
7 years ago

  • Keywords commit needs-refresh removed

It looks like we already allow wildcard subdomains for some other providers (eg Imgur, Tumblr, Slideshare) so we'll just go with that.

I'll update and commit.

#49 @johnbillion
7 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 41615:

Embeds: Add support for Amazon Kindle instant previews.

Props jsepia, morganestes, adamsilverstein, swissspidy, jbpaul17, johnbillion, rugved

Fixes #38181

#50 @johnbillion
7 years ago

In 41616:

Embeds: Documentation alignment following [41615].

See #38181

#51 @johnbillion
7 years ago

In 41676:

Embeds: Alignment following [41615].

See #38181

This ticket was mentioned in Slack in #meta by spacedmonkey. View the logs.


6 years ago

Note: See TracTickets for help on using tickets.