Making WordPress.org

Opened 9 years ago

Closed 9 years ago

#1459 closed defect (bug) (fixed)

WP oEmbeds don't work on WordCamp.org

Reported by: iandunn's profile iandunn Owned by: iandunn's profile iandunn
Milestone: Priority: normal
Component: WordCamp Site & Plugins Keywords: good-first-bug has-patch commit
Cc:

Description

Trying to embed a WordCamp.org post via oEmbed doesn't work. The {post_url}/embed endpoint outputs the content, but the client doesn't actually embed it.

See https://wordpress.slack.com/archives/meta-wordcamp/p1449782802000037

Attachments (3)

1459.patch (1.3 KB) - added by SergeyBiryukov 9 years ago.
meta-1459.2.patch (551 bytes) - added by SergeyBiryukov 9 years ago.
meta-1459.3.patch (1.0 KB) - added by prettyboymp 9 years ago.
Proposal to automatically whitelist any registered namespaces to wp-api v2

Download all attachments as: .zip

Change History (15)

#1 @Otto42
9 years ago

The json+oembed endpoints are returning incorrect results.

https://sandiego.wordcamp.org/2016/wp-json/oembed/1.0/embed?url=https%3A%2F%2Fsandiego.wordcamp.org%2F2016%2Fwelcome-to-the-new-wordcamp-san-diego-2016-website%2F

[{"code":"json_no_route","message":"No route was found matching the URL and request method"}]

#3 @iandunn
9 years ago

  • Owner set to iandunn
  • Status changed from new to accepted

Yup, that'd be my guess too.

@SergeyBiryukov
9 years ago

#4 @SergeyBiryukov
9 years ago

  • Keywords has-patch added; needs-patch removed

The issue is that WP-API v1.2.4 does not support the WordPress 4.4 embed functionality:
https://wordpress.org/support/topic/wp-rest-api-124-breaks-embed-functionality-of-wp-44

Perhaps we could map the route so that the oEmbed API endpoint would be handled by core?

1459.patch fixes the issue in my testing.

This ticket was mentioned in Slack in #core-restapi by dd32. View the logs.


9 years ago

#6 follow-up: @iandunn
9 years ago

I couldn't get this working on my wordcamp.org sandbox; it looks like wp_filter_oembed_result() is returning false because the iframe is stripped out of $html by wp_kses(). That doesn't make sense, since it's in $allowed_tags, but maybe something else in my environment (and unrelated to this patch) is interfering. I'll take another look when I have some more time.

#7 in reply to: ↑ 6 @SergeyBiryukov
9 years ago

Replying to iandunn:

I'll take another look when I have some more time.

FWIW, I tested with WP Meta Environment.

#8 @SergeyBiryukov
9 years ago

  • Keywords commit added

Refreshed the patch after [2358], which basically implements the same idea.

#9 @ryelle
9 years ago

I saw marktimemedia mention this in Slack today, so I went debugging (totally missed iandunn's follow up/this ticket) and came up with the same solution as SergeyBiryukov (also using the meta environment). Is there anything I can do to help test this more?

#10 @iandunn
9 years ago

I still haven't been able to get it working in my sandbox, and I also tested meta-1459.2.patch in production for awhile since it's unlikely to cause any problems, but I couldn't get it working there either.

https://wordpress.slack.com/archives/meta-wordcamp/p1454026616000074

I'm not sure why it's working with your copy of the Meta Environment and also Sergey's, but not my sandbox or production. I don't have any ideas of what to suggest since it's working for you locally. I'll probably just have to dig into comment:6 again when I have some time.

If you have any ideas or suggestions, though, I'm definitely open to trying other things.

@prettyboymp
9 years ago

Proposal to automatically whitelist any registered namespaces to wp-api v2

#11 @iandunn
9 years ago

In 3104:

WordCamp JSON API: Automatically route all v2 requests to the Core handler.

See #1459
Props prettyboymp

#12 @iandunn
9 years ago

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

Great idea @prettyboymp :)

It also looks like the original oEmbed issue is fixed now too.

Note: See TracTickets for help on using tickets.