WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#10337 closed task (blessed) (fixed)

Easier embeds for 2.9 (oEmbed perhaps?)

Reported by: ryan Owned by: Viper007Bond
Milestone: 2.9 Priority: normal
Severity: normal Version:
Component: Shortcodes Keywords: has-patch needs-testing
Focuses: Cc:

Description

We could include the top 10 from wp.com as a start.

Attachments (28)

10337.diff (11.3 KB) - added by ryan 5 years ago.
oembed.php (7.7 KB) - added by Viper007Bond 5 years ago.
Shortcode-based oEmbed proof-of-concept plugin by mdawaffe
reverseable-shortcodes.tar.gz (5.7 KB) - added by Viper007Bond 5 years ago.
Reversable shortcodes allowing non-unfiltered_html user to paste certain embed HTMLs and have it be stored in DB as a shortcode
10337.mockup.patch (44.5 KB) - added by Viper007Bond 5 years ago.
Working mockup code, a very work in progress
10337.workinprogress.patch (47.2 KB) - added by Viper007Bond 5 years ago.
Cache oEmbed results to post meta on post save, XML support, and more
json.php4.patch (37.8 KB) - added by Viper007Bond 5 years ago.
This patch is required for all future patches if you're running PHP4 (future patches won't have this JSON stuff included)
10337.workinprogress.2.patch (13.0 KB) - added by Viper007Bond 5 years ago.
PollDaddy + other stuff
10337.workinprogress.3.patch (14.0 KB) - added by Viper007Bond 5 years ago.
Move oEmbed caching to an AJAX request after the post is saved
10337.workinprogress.4.patch (23.1 KB) - added by Viper007Bond 5 years ago.
Nearing initial commit worthy
test-content.txt (1.1 KB) - added by scribu 5 years ago.
tests for youtube and dailymotion
test-content.2.txt (1.1 KB) - added by scribu 5 years ago.
comprehensive test cases
10337.patch (23.6 KB) - added by Viper007Bond 5 years ago.
Further improvements, especially to the autoembed feature. Should pass scribu's test case.
test-content.3.txt (1.5 KB) - added by scribu 5 years ago.
10337.2.patch (24.9 KB) - added by Viper007Bond 5 years ago.
Revert bad attempt at preventing link embedding (see comment) + options page improvements (using oEmbed is an option now)
10337.3.patch (31.1 KB) - added by Viper007Bond 5 years ago.
Countless improvements, phpdocs, and more
10337.5.patch (33.8 KB) - added by Viper007Bond 5 years ago.
10337.youtubeoembedandtweaks.patch (6.2 KB) - added by Viper007Bond 5 years ago.
Switches YouTube to using oEmbed, adds easier way to add a new oEmbed provider URL, and some phpdoc improvements
10337.options.patch (2.2 KB) - added by Viper007Bond 5 years ago.
Tweak the settings page a bit
10337.6.patch (4.1 KB) - added by Viper007Bond 5 years ago.
Allow non-admins to use Vimeo and allow regex in oEmbed URL formats
10337.7.patch (8.0 KB) - added by Viper007Bond 4 years ago.
Needs a DB ver bump. Switch oEmbed option to control discovery. Tweak whitelist and settings page.
10337.8.patch (1.1 KB) - added by Viper007Bond 4 years ago.
Fix malformed regex (whoops!)
10337.9.patch (5.8 KB) - added by Viper007Bond 4 years ago.
Add a bunch of new filters. Use one of them to remove new lines from Scribd embeds. Invalid oEmbed post meta cache without using JS.
10337.10.patch (1.6 KB) - added by Viper007Bond 4 years ago.
On theme switch, delete all oEmbed caches for all posts as different themes have different widths.
10337.11.patch (2.4 KB) - added by Viper007Bond 4 years ago.
Whitelist WordPress.tv's new oEmbed provider
10337-dailymotion-oembed.patch (4.2 KB) - added by Viper007Bond 4 years ago.
Change DailyMotion from internal handler to using oEmbed (yay!)
10337.12.patch (1.6 KB) - added by Viper007Bond 4 years ago.
Don't use the object cache. We can't be sure it won't expire and that will require re-caching on a page load (that's bad). Always use post meta API (it has it's own object caching). Also use better cache key.
10337.2.diff (4.0 KB) - added by ryan 4 years ago.
Remove discover option. Require plugin to enable it.
10337.13.patch (447 bytes) - added by Viper007Bond 4 years ago.
Disable discovery in the oEmbed class by default

Download all attachments as: .zip

Change History (107)

comment:1 ryan5 years ago

Adds youtube, flickr, polldaddy, and WP archives shortcodes. Needs some cleanup, but that's a start.

comment:2 ryan5 years ago

  • Summary changed from Bundle shortcodes for common services to Bundle shortcodes for popular sites, services, and template tags.

comment:3 ryan5 years ago

The youtube shortcode doesn't match the syntax of the wp.com youtube shortcode. The wp.com shortcode uses the old Faux ML stuff rather than the shortcode API. I need to see if I can get the Faux ML syntax working with shortcodes.

ryan5 years ago

comment:4 greenshady5 years ago

Wouldn't this stuff be better relegated to the plugins directory? I don't see why shortcodes for YouTube, Flickr, and Poll Daddy should be integrated into core. If anything, I don't think this should go beyond a bundled plugin.

comment:5 husobj5 years ago

I don't object to these common services being easy to embed in WordPress out-of-the-box, but will plugins be able to override these shortcodes if desired?

comment:6 dd325 years ago

I don't object to these common services being easy to embed in WordPress out-of-the-box, but will plugins be able to override these shortcodes if desired?

Most definately, IIRC, The last registered shortcode will be the one used.. Pretty sure theres an API for de-registering shortcodes too, so plugins could use that before registering their own.

comment:7 follow-up: Viper007Bond5 years ago

  • Owner set to Viper007Bond
  • Status changed from new to assigned
  • Summary changed from Bundle shortcodes for popular sites, services, and template tags. to Easier embeds for 2.9 (oEmbed perhaps?)

This was a widely requested feature in the 2.9 feature poll:

http://wordpress.org/development/2009/07/2-9-vote-results/

Since I've spent so much of my life dealing with embeds, Jane asked me take a look at implementing easier embeds.

I've been thinking about it over the past week or two and I'm currently of the opinion that implementing an oEmbed client with some manually coded fallbacks for popular sites that don't support oEmbed yet would be the way to go.

My Current Brainstorm

New UI element is added, perhaps a new media button called like External Embed or something. I dunno, UI's aren't my strong suit.

The user is prompted for the URL to the object (video, image, etc.).

WordPress then...

  1. Connects to the URL looking in the head for a discovery <link> to their oEmbed provider.
  2. If not found, it checks the URL against an internal (filterable) list of oEmbed providers. This is meant for sites that support oEmbed but that don't have discovery links.
  3. If still not found, then it checks the URL against and internal sudo-oEmbed provider. Similar http://oohembed.com/ but it'd be internal rather than relying on a third party service. This is meant for popular sites that we want to support but that don't support oEmbed themselves (for example YouTube).

mdawaffe and I have been discussing this and the only real concern seems to be malicious HTML getting in from the provider (say hidden tracking codes, advertising, etc.) However this would still happen if the user was to just copy/paste in the code themselves.

There's also the decision about whether to do this with shortcodes or not.

The before method consists of the oEmbed result HTML being inserted directly into the post contents. The user can then see the HTML and remove/edit anything they want, if they desire. The drawback is if the provider updates their embed code, it's not updated on the blog. Although this can be a pro if the update consists of malicious HTML. The other problem is it's harder to change things like width/height as it's often defined multiple times.

The after (shortcode) method consists of a shortcode being inserted into the post rather than the HTML. The oEmbed request is made on post display (and probably cached for performance purposes). This is future proof and probably easier on the user, but the user has no control over the HTML. This does allow unfiltered_html lacking users (i.e. WPMU) to embed though, which is both a pro and con.

Lack Of Customization

Another problem with oEmbed is the lack of customization. For example, you couldn't easily set a default color for Vimeo embeds.

TL;DR

I am thinking about using oEmbed for making for easy embeds, but I am concerned about possible security issues, lack of customization, and so forth. As such, I am looking for feedback and ideas.

Viper007Bond5 years ago

Shortcode-based oEmbed proof-of-concept plugin by mdawaffe

Viper007Bond5 years ago

Reversable shortcodes allowing non-unfiltered_html user to paste certain embed HTMLs and have it be stored in DB as a shortcode

comment:8 Viper007Bond5 years ago

Reverseable Shortcodes is by mdawaffe as well.

comment:9 in reply to: ↑ 7 ; follow-up: lloydbudd5 years ago

Replying to Viper007Bond:

This was a widely requested feature in the 2.9 feature poll:

http://wordpress.org/development/2009/07/2-9-vote-results/

I'm excited by this work!

I do think an important context of the voting results is that WordPress.com customers (a restricted WordPress MU environment with limited embed options) were called to vote: http://en.blog.wordpress.com/2009/07/08/media-feature-vote/

I've been thinking about it over the past week or two and I'm currently of the opinion that implementing an oEmbed client with some manually coded fallbacks for popular sites that don't support oEmbed yet would be the way to go.

My Current Brainstorm

New UI element is added, perhaps a new media button called like External Embed or something. I dunno, UI's aren't my strong suit.

Strengthen the muscle by working it ;-)

There is a UI for this in WordPress already, but I don't think it works as many would expect.

All of the Upload/Insert include From URL, so for Video I go there and enter a YouTube URL thinking it will add that video to the post, but instead it currently (v2.8.x and trunk r11731) just creates a link to the URL with the text you enter as the title.

The user is prompted for the URL to the object (video, image, etc.).

WordPress then...

  1. Connects to the URL looking in the head for a discovery <link> to their oEmbed provider.
  2. If not found, it checks the URL against an internal (filterable) list of oEmbed providers. This is meant for sites that support oEmbed but that don't have discovery links.
  3. If still not found, then it checks the URL against and internal sudo-oEmbed provider. Similar http://oohembed.com/ but it'd be internal rather than relying on a third party service. This is meant for popular sites that we want to support but that don't support oEmbed themselves (for example YouTube).

mdawaffe and I have been discussing this and the only real concern seems to be malicious HTML getting in from the provider (say hidden tracking codes, advertising, etc.)

However this would still happen if the user was to just copy/paste in the code themselves.

Correct, though expectations sometimes change when there is a different experience.

There's also the decision about whether to do this with shortcodes or not.

The before method consists of the oEmbed result HTML being inserted directly into the post contents. The user can then see the HTML and remove/edit anything they want, if they desire. The drawback is if the provider updates their embed code, it's not updated on the blog. Although this can be a pro if the update consists of malicious HTML. The other problem is it's harder to change things like width/height as it's often defined multiple times.

I think you nail the issue of the experience not being that much better than pasting in the embed code oneself.

The after (shortcode) method consists of a shortcode being inserted into the post rather than the HTML. The oEmbed request is made on post display (and probably cached for performance purposes). This is future proof and probably easier on the user, but the user has no control over the HTML. This does allow unfiltered_html lacking users (i.e. WPMU) to embed though, which is both a pro and con.

Lack Of Customization

Another problem with oEmbed is the lack of customization. For example, you couldn't easily set a default color for Vimeo embeds.

TL;DR

I am thinking about using oEmbed for making for easy embeds, but I am concerned about possible security issues, lack of customization, and so forth. As such, I am looking for feedback and ideas.

How it fits into the existing roles and permissions system needs to be vetted.

Also important will be clear, immediate feedback for a web service that is not supported and generic instructions for copy & pasting embed code provided.

I think oEmbed is a great default experience, but if technically feasible would be great to have these short codes of a little different style, with traditional short code taking precedence if already loaded (would have to extend their definitions).

For example, user pastes link to Flickr video:

  1. Check if traditional short code in system matching that definition. Can provide explicit fields for the possible parameters defined for the short code (addresses MU, security, and customization/flexibility).
  1. If unfiltered HTML cap. then use oembed stages you describe above.

comment:10 in reply to: ↑ 9 mdawaffe5 years ago

Replying to lloydbudd:

I think oEmbed is a great default experience, but if technically feasible would be great to have these short codes of a little different style, with traditional short code taking precedence if already loaded (would have to extend their definitions).

For example, user pastes link to Flickr video:

  1. Check if traditional short code in system matching that definition. Can provide explicit fields for the possible parameters defined for the short code (addresses MU, security, and customization/flexibility).
  1. If unfiltered HTML cap. then use oembed stages you describe above.

The Proof of Concept plugin attached above prefers predefined shortcodes over oEmbed. If it notices that there's an already defined shortcode for the resource I'm trying to oEmbed, it uses the shortcode, not the oEmbed stuff.

I'm talking about the plugin's backend stuff. There's no UI at all in the proof of concept plugin.

comment:11 Viper007Bond5 years ago

Updated plan is shortcodes. Custom, "hard coded" shortcodes for widely used sites such as YouTube. This allows extended options like colors, etc. that you can't control with oEmbed.

Falls back to oEmbed for unsupported sites, but the oEmbed shortcode is capability controlled. Requires that the author have unfiltered_html cap, else they could run their own oEmbed provider and inject "any" HTML they want (even if it's kses'ed, they could still do bad stuff via Flash).

Viper007Bond5 years ago

Working mockup code, a very work in progress

Viper007Bond5 years ago

Cache oEmbed results to post meta on post save, XML support, and more

comment:12 Viper007Bond5 years ago

New patch attached in order to solicit feedback. It's not ready for the core yet, but it's moving in that direction.

comment:13 Viper007Bond5 years ago

Todo:

  • Customization via shortcode attributes (about all you can do right now is width/height)
  • More built-in handlers perhaps (like YouTube's is currently)

Viper007Bond5 years ago

This patch is required for all future patches if you're running PHP4 (future patches won't have this JSON stuff included)

Viper007Bond5 years ago

PollDaddy + other stuff

Viper007Bond5 years ago

Move oEmbed caching to an AJAX request after the post is saved

comment:14 azaozz5 years ago

(In [11875]) Add JSON compat for PHP < 5.2, props Viper007Bond, see #10337

Viper007Bond5 years ago

Nearing initial commit worthy

comment:15 Viper007Bond5 years ago

  • Keywords needs-testing added

comment:16 scribu5 years ago

Applied 10337.workinprogress.4.patch, but nothing happens.

This is the content I used:

http://www.youtube.com/watch?v=YkimIxWne30

<a href="#">http://www.youtube.com/watch?v=YkimIxWne30</a>

[youtube]http://www.youtube.com/watch?v=YkimIxWne30[/youtube]

comment:17 Viper007Bond5 years ago

From the meeting today:

[Thu 14:21:43] <Viper007Bond> scribu: oh, it's [embed] not [youtube] and to enable automatic embedding of plain urls outside of shortcodes you need to visit settings -> media
[Thu 14:21:56] <Viper007Bond> it's off by default due to the agressiveness of the feature
[Thu 14:22:03] <scribu> Viper007Bond: got it

And currently that second link will get embedded (it shouldn't) if you have auto-embeds on, but I thought up a solution last night in bed. Expect yet another patch soon.

scribu5 years ago

tests for youtube and dailymotion

scribu5 years ago

comprehensive test cases

comment:18 scribu5 years ago

Ok, so, with auto-embed activated:

Patch: 10337.workinprogress.4.patch

Test: test-content.txt

Results:

Youtube
1 - pass (embed)
2 - pass (embed)
3 - pass (text)
4 - pass (embed)
5 - pass (embed)
6 - fail (embed)

Dailymotion
7 - pass (embed)
8 - pass (embed)
9 - pass (text)
10 - pass (embed)
11 - pass (embed)
12 - fail (embed)

Other
13 - pass (text)
14 - fail (Parse error: syntax error, unexpected '<' in /wp-includes/class-oembed.php on line 175)
15 - fail (Parse error: syntax error, unexpected '<' in /wp-includes/class-oembed.php on line 175)

comment:19 Viper007Bond5 years ago

6 - known issue, fix in the works.
14/15 - line 175 is blank and I've never seen that error before. I'll look into it.

Also your test content only tests the internal handlers. Here's two URLs you can use to test oEmbed:

http://www.hulu.com/watch/56632/saturday-night-live-digital-short-im-on-a-boat

Hulu does not support discovery, so it's provider URL is included in the WP_oEmbed class.

http://www.vimeo.com/240975

Vimeo DOES support discover (it's provider URL is in it's <head>). It's a good test of an "unknown" URL type.

Viper007Bond5 years ago

Further improvements, especially to the autoembed feature. Should pass scribu's test case.

scribu5 years ago

comment:20 scribu5 years ago

  • Cc scribu@… added

The Parse error was my fault: the oembed class was duplicated.

I've applied the new patch, but the auto-embed doesn't work at all, with the option checked.

I've added a few test for Vimeo and Hulu, as well.

Revision: 11893

Patch: 10337.patch

Test: test-content.3.txt

Youtube
1 - fail (text)
2 - fail (text)
3 - pass (text)
4 - pass (embed)
5 - pass (embed)
6 - pass (link)

Dailymotion
7 - fail (text)
8 - fail (text)
9 - pass (text)
10 - pass (embed)
11 - pass (embed)
12 - pass (link)

Other
13 - pass (text)
14 - pass (text)
15 - pass (link)

Vimeo
16 - fail (text)
17 - fail (link)
18 - pass (link)

Hulu
19 - fail (text)
20 - pass (embed)
21 - pass (link)

Viper007Bond5 years ago

Revert bad attempt at preventing link embedding (see comment) + options page improvements (using oEmbed is an option now)

comment:21 Viper007Bond5 years ago

The bad attempt I wrote at avoiding embedding <a href="#">url here</a> messed up more than it fixed, so I reverted it. We're back to embedding those (when they shouldn't be). Someone with l33t regex skills will need to modify the regex to fix it up.

I also added using oEmbed as a configuration option (it's on by default for upgrades, but patch users will need to manually enable it due to lack of DB upgrade) and you can leave the width setting blank to use the theme width.

Viper007Bond5 years ago

Countless improvements, phpdocs, and more

comment:22 Viper007Bond5 years ago

Autoembed now appears to be working perfectly thanks to a lot of help from azaozz and DD32. phpdoc has been added. And... some other stuff I'm likely forgetting.

TODO LIST:

  • PHP4 XML parsing in WP_oEmbed. I'm not sure which method to use to parse XML in PHP4.
  • Figure out what to do with feeds. Technically <object> and such breaks feeds. Do we care? Or would we rather allow people using Google Reader and such to view these?
  • Add more default embed handlers. Any suggestions for commonly used media websites?

comment:23 follow-up: ryan5 years ago

Some suggestions:

  • Move default embeds to default-embeds.php
  • Rename getHtml() to get_html()
  • esc_attr() form_option() output in value="" just to be extra safe
  • esc_attr() width and height attributes in handler output

comment:24 in reply to: ↑ 23 Viper007Bond5 years ago

Replying to ryan:

  • esc_attr() form_option() output in value="" just to be extra safe

This is the source of form_option():

echo esc_attr(get_option( $option ) );

So it's already escaped. ;)

comment:25 follow-up: ryan5 years ago

Does the oembed-cache case in admin-ajax.php need to check if current_user_can read the post?

comment:26 ryan5 years ago

And are we going to do an oEmbed whitelist? Right now it looks like oEmbeds are processed on display and the output differs depending on if the current user has unfilter_html or not. We should probably have a whitelist that applies to all users.

comment:27 markjaquith5 years ago

I say we do a whitelist with an array that passes through a filter that plugins can edit.

comment:28 ryan5 years ago

I agree. Get rid of the unfiltered_html check and enforce the whitelist for everyone, accepting embeds only for domains in the whitelist.

comment:29 ryan5 years ago

Per dev meetup, we're not going to bother with PHP4 XML support. json is most often used anyway, so XML support is just a fallback. We'll handle XML for PHP5 and drop PHP4.

comment:30 in reply to: ↑ 25 Viper007Bond5 years ago

Replying to ryan:

Does the oembed-cache case in admin-ajax.php need to check if current_user_can read the post?

Not really. All it does is trigger a silent post rendering which in turn triggers a oEmbed result caching. If this AJAX call doesn't run, then the caching will take place the first time the post is viewed (resulting in a bit of a slow page load).

The unfiltered_html code is bad at the moment as it's checking current user rather than the author of the post. I need to change that.

Using the whitelist for all users would in turn disable the autodiscovery feature (in short, being able to embed sites WordPress doesn't know about). This would increase security as only trusted sites (either by WordPress' approval or by a plugin's approval) could be embeded, but it'd also cut down on the flexibility of the embed feature. Perhaps a filter or something could toggle this feature (I for example know better than to embed bad sites on my blog).

comment:31 follow-up: ryan5 years ago

Let's try the cap check on the post author and an optional whitelist for now. See how that goes for now and switch to whitelist by default if we get nervous. :-)

comment:32 in reply to: ↑ 31 Viper007Bond5 years ago

Replying to ryan:

Let's try the cap check on the post author and an optional whitelist for now. See how that goes for now and switch to whitelist by default if we get nervous. :-)

Sounds good. Meant to code this yesterday, but ran out of time. Hoping to find some time today between packing sessions.

Viper007Bond5 years ago

comment:33 Viper007Bond5 years ago

Adds author_can() function where you can pass a post and a capability. Works similarly to current_user_can(). If the author of the post doesn't have unfiltered_html, then oEmbed discovery is disabled and only known providers (via the filterable array) can be used.

Also moves default handlers to default-embeds.php and escapes all variables just to be extra safe.

comment:34 ryan5 years ago

(In [12023]) Embeds. Props Viper007Bond. see #10337

comment:35 Viper007Bond5 years ago

Todo before 2.9 release: uncomment Vimeo's oEmbed provider URL from class-oembed.php. It's currently commented out as Vimeo is one of the very few sites that supports discovery (via a <link>), so it's a good site to use as a test of the discovery code/feature. Drawback of having it commented out is non-admins can't embed Vimeo currently. ;)

comment:36 follow-up: ryan5 years ago

Can we have Vimeo in the whitelist and still have it do discovery?

Also, should this be commented out?

                //case 'embed_size_w':

comment:37 follow-up: Otto425 years ago

Question: YouTube does support oEmbed. Even has discovery. So, why a special handler for it?

oEmbed link:
http://www.youtube.com/oembed?url=http%3A//www.youtube.com/watch?v%3D-UUx10KOWIE&format=xml

And in the source of the video's html page here:
http://www.youtube.com/watch?v=-UUx10KOWIE

You find this:

<link rel="alternate" type="application/json+oembed" href="http://www.youtube.com/oembed?url=http%3A//www.youtube.com/watch?v%3D-UUx10KOWIE&format=json" title="Jude Law:  My Take On Peace Video Contest" />
<link rel="alternate" type="text/xml+oembed" href="http://www.youtube.com/oembed?url=http%3A//www.youtube.com/watch?v%3D-UUx10KOWIE&format=xml" title="Jude Law:  My Take On Peace Video Contest" />

comment:38 in reply to: ↑ 36 Viper007Bond5 years ago

Replying to ryan:

Can we have Vimeo in the whitelist and still have it do discovery?

No. Discovery is only used if the URL does not match any of the predefined oEmbed providers (that array that's also used as a whitelist of sorts).

Also, should this be commented out?

                //case 'embed_size_w':

Yes, and infact it can probably be removed. It's commented out as you can leave the field blank at Settings -> Media and it will default to your theme's $content_width or if that isn't set, then some pre-defined default. If that's uncommented, then it'll end up being 0 rather than staying blank.

A custom sanitizer should probably be used that allows blank OR int and nothing else (the value is currently int'ed before use, but no sense in having an invalid data type be able to show up in that field).

comment:39 in reply to: ↑ 37 Viper007Bond5 years ago

Replying to Otto42:

Question: YouTube does support oEmbed. Even has discovery. So, why a special handler for it?

Because YouTube added oEmbed support literally 4 days ago and I was unaware that they had (I've been out of town since Saturday).

http://apiblog.youtube.com/2009/10/oembed-support.html

Incoming patch soon. :)

Viper007Bond5 years ago

Switches YouTube to using oEmbed, adds easier way to add a new oEmbed provider URL, and some phpdoc improvements

comment:40 follow-up: ryan5 years ago

Mind if I rename wp_oembed_addprovider to wp_oembed_add_provider?

comment:41 automattor5 years ago

(In [12027]) Use oEmbed for youtube. Props Viper007Bond. see #10337

comment:42 in reply to: ↑ 40 Viper007Bond5 years ago

Replying to ryan:

Mind if I rename wp_oembed_addprovider to wp_oembed_add_provider?

Yeah, that's fine. My rational was using underscores as seperators (wordpress_subcategory_function) but an extra underscore makes it easier to read. :)

comment:43 filosofo5 years ago

It's worth pointing out that the vast majority of WordPress functions (about 98%, last time I checked) have underscores between all the words. Being consistent that way with future functions will make it possible to remember how they're supposed to be spelled (I'm looking at you, update_usermeta).

Viper007Bond5 years ago

Tweak the settings page a bit

comment:44 ryan5 years ago

(In [12076]) Tweak embed settings. Props Viper007Bond. see #10337

Viper007Bond5 years ago

Allow non-admins to use Vimeo and allow regex in oEmbed URL formats

comment:45 ryan5 years ago

(In [12096]) Allow non-admins to use Vimeo and allow regex in oEmbed URL formats. Props Viper007Bond. see #10337

comment:46 follow-up: Otto425 years ago

Minor youtube embed issue, it doesn't handle the non-www version anymore, unless you have unfiltered-html.

Quick fix: Replace the youtube provider info with this:

'#http://(www\.)?youtube\.com/watch.*#i'   => array( 'http://www.youtube.com/oembed', true)

comment:47 in reply to: ↑ 46 Viper007Bond5 years ago

Replying to Otto42:

Minor youtube embed issue, it doesn't handle the non-www version anymore, unless you have unfiltered-html.

Non-WWW YouTube URLs don't exist. Visit this URL:

http://youtube.com/watch?v=hl2UUunlI2Q

You'll be redirected to WWW.

However I guess it's worth searching for them anyway.

comment:48 azaozz5 years ago

(In [12098]) When inserting the embed shortcode in TinyMCE replace it with a placeholder image, update the 'wordpress', 'wpeditimage' and 'wpgallery' TinyMCE plugins, fix the audio and video "From URL" tabs in the uploader popup to accept only URLs for embedding, see #10337

comment:49 ryan4 years ago

(In [12123]) Fix revision3 scheme. Add photobucket oembed. see #10337

comment:50 ryan4 years ago

(In [12131]) Add scribd to oembed provider whitelist. see #10337

Viper007Bond4 years ago

Needs a DB ver bump. Switch oEmbed option to control discovery. Tweak whitelist and settings page.

comment:51 ryan4 years ago

(In [12136]) Switch oEmbed option to control discovery. Tweak whitelist and settings page. Props Viper007Bond. see #10337

Viper007Bond4 years ago

Fix malformed regex (whoops!)

comment:52 ryan4 years ago

(In [12138]) Fix regex. Props Viper007Bond. see #10337

Viper007Bond4 years ago

Add a bunch of new filters. Use one of them to remove new lines from Scribd embeds. Invalid oEmbed post meta cache without using JS.

comment:53 ryan4 years ago

(In [12153]) Add a bunch of new filters. Use one of them to remove new lines from Scribd embeds. Invalid oEmbed post meta cache without using JS. Props Viper007Bond. see #10337

Viper007Bond4 years ago

On theme switch, delete all oEmbed caches for all posts as different themes have different widths.

comment:54 ryan4 years ago

I'm kinda scared to see how those two queries behave with a big posts table. :-) And cache delete loops can be pretty slow.

comment:55 Otto424 years ago

Viper007Bond: Question about the patch:

Why this?

$postmetaids = $wpdb->get_col( "SELECT meta_id FROM $wpdb->postmeta WHERE meta_key LIKE '_oembed_%'" );
$in = implode( ',', array_fill( 1, count($postmetaids), '%d' ) );  
$wpdb->query( $wpdb->prepare( "DELETE FROM $wpdb->postmeta WHERE meta_id IN($in)", $postmetaids ) );

Why not this?

DELETE FROM $wpdb->postmeta WHERE meta_key LIKE '_oembed_%'

Only reason I can see is for the surrounding actions, which also seem somewhat unnecessary to me. A basic action there would be enough, if the action function needs the meta id's, it can do its own select.

comment:56 Viper007Bond4 years ago

I was merely going off existing code. Take a look at delete_post_meta_by_key(). It's the exact same thing, just no LIKE.

Viper007Bond4 years ago

Whitelist WordPress.tv's new oEmbed provider

comment:57 ryan4 years ago

(In [12181]) Add wordpress.tv to oEmbed provider whitelist. Props Viper007Bond. see #10337

comment:58 follow-up: sirzooro4 years ago

YouTube has many native versions, e.g. pl.youtube.com in Polish. Change whitelist rule to include them too.

comment:59 in reply to: ↑ 58 Viper007Bond4 years ago

Replying to sirzooro:

YouTube has many native versions, e.g. pl.youtube.com in Polish. Change whitelist rule to include them too.

Not anymore they don't. Click and see for yourself: http://pl.youtube.com/

It redirects to http://www.youtube.com/?gl=PL&hl=pl

They've moved all content to www.youtube.com (that exact URL).

comment:60 Denis-de-Bernardy4 years ago

Could we add a class in there in a way or another, and ensure objects don't get wrapped in p tags?

I just tried:

http://www.youtube.com/watch?v=hl2UUunlI2Q

Works wonders, *but* I got this code:

<p><object...

instead of the expected:

<div class="oembed oembed-youtube"><object...

comment:61 follow-up: Viper007Bond4 years ago

The paragraph tags are on purpose. Without them, the object would be crammed against the text around it.

The <div> is an interesting idea (it'd allow you to like put a box around your embeds), but it's not really easily possible to add the "youtube" bit (the whole idea is for WordPress not to handle URLs on a case-by-case basis).

I'll fool around with some ideas though for adding increased flexibility.

comment:62 in reply to: ↑ 61 Mendezki4 years ago

I've noticed the width="" property doesn't seem to function properly for youtube videos. The height parameter works as expected.

comment:64 lloydbudd4 years ago

Related tickets:
#11288 oEmbed should be removed from the UI -- admin (editor)
#11319 oEmbed should be integrated into Add ... from URL experiences

Viper007Bond4 years ago

Change DailyMotion from internal handler to using oEmbed (yay!)

Viper007Bond4 years ago

Don't use the object cache. We can't be sure it won't expire and that will require re-caching on a page load (that's bad). Always use post meta API (it has it's own object caching). Also use better cache key.

comment:65 Viper007Bond4 years ago

Two new patches. One switches DailyMotion support from a hard-coded handler to oEmbed. The other ditches the usage of the object cache as it can't be expected to never expire (we don't want caching without the blog owner's knowledge) and there's also no easy way to clear out the oEmbed cache for a post. Better to just stick to using the post meta.

comment:66 scribu4 years ago

  • Keywords has-patch added

comment:67 ryan4 years ago

(In [12325]) Use oEmbed for dailymotion. Props Viper007Bond. see #10337

comment:68 ryan4 years ago

(In [12326]) Cleanup oembed caching. Props Viper007Bond. see #10337

comment:69 ryan4 years ago

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

ryan4 years ago

Remove discover option. Require plugin to enable it.

Viper007Bond4 years ago

Disable discovery in the oEmbed class by default

comment:70 follow-up: navjotjsingh4 years ago

Why the oEmbed has been disabled now? Ordinary users will find it frustrating that even when WP 2.9 promises oEmbeds, it is still not enabled and available?

comment:71 follow-up: Otto424 years ago

Fix for the now-broken oEmbed functionality:

add_filter('embed_oembed_discover', create_function('$a','return true;'));

I'm very disappointed that the team decided to cripple this otherwise extremely cool functionality.

comment:72 navjotjsingh4 years ago

Thanks for the fix. Was looking for it. Even I am disappointed at this step. Thousands of users who don't understand PHP and how to enable it, may find themselves possibly unknown of how to embed a video directly when WP 2.9 promised it months earlier.

comment:73 in reply to: ↑ 70 Viper007Bond4 years ago

Replying to navjotjsingh:

Why the oEmbed has been disabled now? Ordinary users will find it frustrating that even when WP 2.9 promises oEmbeds, it is still not enabled and available?

oEmbed is not disabled, it's merely limited to a whitelist for security purposes.

And is it really that hard to install a plugin?

http://wordpress.org/extend/plugins/enable-oembed-discovery/

comment:74 in reply to: ↑ 71 ; follow-up: Viper007Bond4 years ago

Replying to Otto42:

I'm very disappointed that the team decided to cripple this otherwise extremely cool functionality.

We're saving people from themselves. It's not "crippled" as you call it, it's just limited to a whitelist so the novice user doesn't screw themselves over.

If a user happened to paste the URL to something on my blog on it's own line and unhyperlinked, I could easily make that URL turn into a bit of code that'd steal their login cookies without them ever knowing. Then I'd have full access to their blog.

comment:75 in reply to: ↑ 74 ; follow-up: Otto424 years ago

Replying to Viper007Bond:

We're saving people from themselves. It's not "crippled" as you call it, it's just limited to a whitelist so the novice user doesn't screw themselves over.

Limiting it to a whitelist of sites is what makes it "crippled". The whole point of oEmbed is to not be limited to selected sites, but to work with any site.

Without discovery, oEmbed is basically unnecessary, you can just use a plugin to add compatibility for each site using whatever methods that site supports.

If a user happened to paste the URL to something on my blog on it's own line and unhyperlinked, I could easily make that URL turn into a bit of code that'd steal their login cookies without them ever knowing. Then I'd have full access to their blog.

There's better ways. Even says so in the oEmbed document itself: "Consumers may wish to load the HTML in an off-domain iframe to avoid XSS vulnerabilities."

comment:76 in reply to: ↑ 75 scribu4 years ago

Replying to Otto42:

There's better ways. Even says so in the oEmbed document itself: "Consumers may wish to load the HTML in an off-domain iframe to avoid XSS vulnerabilities."

That might work. Please open a new ticket for 3.0.

comment:77 follow-up: Viper007Bond4 years ago

So what, proxy all video HTML through WordPress.org? We can't exactly be expected to do that.

comment:78 in reply to: ↑ 77 ; follow-up: otto424 years ago

Replying to Viper007Bond:

So what, proxy all video HTML through WordPress.org? We can't exactly be expected to do that.

Of course not, that would be insane. But there's got to be a better way than whitelisting. Even doing something like stripping out script tags by default would be a better solution.

comment:79 in reply to: ↑ 78 Viper007Bond4 years ago

Replying to otto42:

Of course not, that would be insane. But there's got to be a better way than whitelisting. Even doing something like stripping out script tags by default would be a better solution.

I thought about that, but a lotta sites do their embeds using Javascript. And even <object> itself can be bad (there's a reason none of that stuff is allowed on WordPress.com for example).

Note: See TracTickets for help on using tickets.