WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 19 months ago

Last modified 7 months ago

#16996 closed enhancement (fixed)

oEmbed: Allow custom arguments to be specified

Reported by: newmediarts Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.1
Component: Embeds Keywords: has-patch needs-testing
Focuses: Cc:

Description

Vimeo, and likely other providers as well, allow custom arguments to be passed when fetching the oEmbed code to customize the returned player HTML (e.g. color, autoplay, etc.).

Currently, it seems only the maxheight and maxwidth arguments are passed when fetching the oEmbed data.

However, I'd like to do something like this:

http://vimeo.com/12345?color=7AB800

#=> http://vimeo.com/api/oembed.xml?url=http%3A//vimeo.com/12345&color=7AB800

which will let me customize the player colour.

Attachments (6)

16996.diff (818 bytes) - added by jacobwg 3 years ago.
Initial patch to allow for any custom [embed] parameters to be passed on to the oEmbed request
16996-2.diff (1.2 KB) - added by jacobwg 3 years ago.
16996-3.diff (556 bytes) - added by r-a-y 3 years ago.
16996-4.diff (562 bytes) - added by r-a-y 3 years ago.
Passes $url as a parameter
16996-5.diff (1.3 KB) - added by jacobwg 3 years ago.
Passes custom params from shortcode and URL - we need to check to see if we need to filter these params (it also currently passes the discover arg to the request which the oEmbed provider ignores)
16996-filter.diff (645 bytes) - added by Otto42 23 months ago.
Simpler filter which makes the parameters easily pluggable

Download all attachments as: .zip

Change History (33)

comment:1 newmediarts3 years ago

  • Cc gabrielmansour@… added

jacobwg3 years ago

Initial patch to allow for any custom [embed] parameters to be passed on to the oEmbed request

comment:2 jacobwg3 years ago

  • Cc jacobwg added
  • Keywords has-patch added

I've added a patch that just passes on any custom shortcode params to the oEmbed request - I had to do some unsetting as the oEmbed maxwidth and maxheight are called width and height.

comment:3 newmediarts3 years ago

Thanks for the initial patch, jacobwg!

Instead of using a foreach statement, add_query_arg() lets you pass in an associative array of parameters, so you can just do this, which is much cleaner:

add_query_arg( $args, $provider );

Would this patch also affect oEmbed URLs that get automatically converted in the post content? That's what I'm most interested in.

comment:4 jacobwg3 years ago

Okay, sorry I haven't gotten back to this yet... I was under the impression that Trac would email me when there were new comments, but since I hadn't told Trac my email address, I don't know how I thought that was going to work. :)

Initially, I don't think that it will affect oEmbed URLs... I'll add a new patch soon with that added.

jacobwg3 years ago

comment:5 jacobwg3 years ago

  • Keywords needs-testing added

New patch that accepts args in the following precedence order:

Shortcode args, URL args, Default oEmbed args

Also, this is for 3.2 as we're using a PHP 5 only function to parse the URL query string. (no 3.2 version option?)

Last edited 3 years ago by jacobwg (previous) (diff)

comment:6 r-a-y3 years ago

You shouldn't be unsetting any of the arguments.

I do agree though that you should be able to pass additional arguments. So I believe a filter is the best option here.

Attached patch.

r-a-y3 years ago

r-a-y3 years ago

Passes $url as a parameter

comment:7 nacin3 years ago

We might need to add in a parameter for YouTube at #17108.

comment:8 newmediarts3 years ago

r-a-y's solution may solve my site's needs, since I'd want to change the player colour globally for this site, so I can just use add_filter to add the addtional params in my theme's functions.php file. However, I don't know if this would solve the greater overarching issue of being able to pass arguments on a per–oEmbed-URL basis.

I'll test try out this patch when I have some free time this week.

comment:9 r-a-y3 years ago

Using the filter, it's possible to add arguments on a per-URL basis (not shortcode) if you use the parse_url() function to see if any arguments (that you want) exist in the URL string.

If they exist, then you could tack them on to $provider. If they don't, don't add them.

comment:10 jacobwg3 years ago

While I agree that a filter should be added, that still doesn't solve the issue for users who do not have access to the code (authors, editors, etc.) but who want to paste in a URI like http://vimeo.com/12345?color=7AB800 and change the color.

jacobwg3 years ago

Passes custom params from shortcode and URL - we need to check to see if we need to filter these params (it also currently passes the discover arg to the request which the oEmbed provider ignores)

comment:11 jacobwg3 years ago

That patch should allow for arguments from the shortcode and the URI, which are passed through a filter for in-code modification of basically anything.

comment:12 r-a-y3 years ago

FYI, if a shortcode is passed with any additional arguments, WP will automatically add them.

You just need to make sure you add any custom arguments (like color) to $provider via the filter in my patch.

comment:13 ramoonus3 years ago

  • Cc ramoonus@… added

comment:14 ramoonus3 years ago

for 3.3?

comment:15 mitchbundy2 years ago

I'd love to see this in WP core asap, but one thing that should be added is something along the lines of default arguments for each provider, because a provider like YouTube should likely have a iframe=1 parameter by default. #17108

Version 0, edited 2 years ago by mitchbundy (next)

comment:16 redwallhp2 years ago

I would also like to see this in core soon. Some oEmbed providers, such as Twitter, push a JavaScript include in the HTML of each embed. They offer a parameter to prevent that if you already have the script included, which many themes and plugins do for the "Retweet" buttons, but the oEmbed API in WordPress doesn't yet make it easy to disable. It would be useful for plugins to be able to not only define default arguments when registering an oEmbed provider, but to be able to modify them via a filter.

(As an aside, I'm somewhat surprised nobody has filed a ticket to include Twitter in the oEmbed whitelist by default...)

comment:17 nacin23 months ago

I thought I've already posted here before, but arbitrary custom arguments can result in security flaws.

oEmbed is about trust — you trust the provider to return safe information. At the moment, the only thing a user can affect is the suggested width and height. More parameters means the possibility of injecting raw CSS, JavaScript, or HTML, all of which would be insecure; or an unsanitized parameter (we've had issues with providers simply sanitizing the widths and heights as integers), etc.

Distinct filters here (if there aren't already one) are probably the best we can do, to allow A) plugins to add more arguments to an oEmbed fetch, and B) plugins to add more accepted arguments for an oEmbed shortcode on a per-provider basis.

Otto4223 months ago

Simpler filter which makes the parameters easily pluggable

comment:18 follow-up: Otto4223 months ago

16996-filter.diff adds the simplest filter to get the job done that I can think of.

With this filter, a plugin could enable arbitrary parameters using code like this:

add_filter('oembed_fetch_url', 'foo', 10, 3);
function foo($provider, $args, $url) {
	foreach ($args as $key=>$value) { 
		switch ($key) { 
		case 'width': 
		case 'height': 
		case 'discover': // ignore these args
			break;
		default: 
			$provider = add_query_arg( $key, $value, $provider ); 
			break; 
		}
	}
	return $provider;
}

Alternatively, it could use the URL param to be more discrete about it. The provider url string is passed whole, so things like remove_query_arg could be used to remove the width, for example (this is somewhat desirable sometimes with Twitter's oembed implementation).

comment:19 DrewAPicture23 months ago

  • Cc xoodrew@… added

comment:20 in reply to: ↑ 18 DrewAPicture23 months ago

Replying to Otto42:

16996-filter.diff adds the simplest filter to get the job done that I can think of.

+1 for the filter. Sometimes changing the global height/width isn't enough and it isn't the best solution for all embeds.

comment:21 nacin20 months ago

  • Milestone changed from Awaiting Review to 3.5
  • Type changed from feature request to enhancement

comment:22 nacin19 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [21839]:

Introduce an oembed_fetch_url filter to be applied before sending the request.

props Otto42, r-a-y. fixes #16996.

comment:23 nacin19 months ago

In [21840]:

Switch $args and $url for the oembed_fetch_url filter introduced in [21839]. A bit more logical, especially given the order of arguments passed to fetch(). see #16996.

comment:24 cliffpaulick18 months ago

  • Cc tko@… added

Non uber-dev here... trying to follow along...

I use SmugMug for embedding videos without "branding" (e.g. the bottom-right corner SmugMug smiley face, the YouTube logo, Vimeo text, etc.). Here's the iframe code they generate for that: <iframe frameborder="0" scrolling="no" width="640" height="360" src="http://api.smugmug.com/services/embed/1344454513_PQSXrW6?width=640&height=360&nologo"></iframe>
See the height and width and nologo parts of the URI?

If I type in
http://media.tourkick.com/Tours/20110615-4116-S-Atlanta-Ave/17612601_LV3TVq
it's just plain text, and if I type in
[embed]http://media.tourkick.com/Tours/20110615-4116-S-Atlanta-Ave/17612601_LV3TVq[/embed]
it just turns into a link (WP 3.4 / 3.5.x beta) even though SmugMug is listed at http://codex.wordpress.org/Embeds#Okay.2C_So_What_Sites_Can_I_Embed_From.3F

So, what am I missing with SmugMug embedding (does it not do video, does it only do photo, does it do both), and how does the &nologo get in the mix with oEmbed (will I need a custom function/plugin or should I just use their generated iframe code instead of relying on oEmbed)?

Thanks for the help understanding all this oEmbed stuff.

For reference: http://wiki.smugmug.net/display/API/oEmbed (it's over my head)

comment:25 follow-up: Otto4218 months ago

@cliffpaulick : This is a question you should ask on the WordPress.org support forums, but "media.tourkick.com" is not "smugmug.com". Try using the actual smugmug.com URL of the video or image file instead.

comment:26 in reply to: ↑ 25 cliffpaulick18 months ago

Replying to Otto42:

@cliffpaulick : This is a question you should ask on the WordPress.org support forums, but "media.tourkick.com" is not "smugmug.com". Try using the actual smugmug.com URL of the video or image file instead.

Thanks, Otto.

media.tourkick.com is a CNAME to smugmug.com so changing the link from http://media.tourkick.com/Tours/20110615-4116-S-Atlanta-Ave/17612601_LV3TVq to http://smugmug.com/Tours/20110615-4116-S-Atlanta-Ave/17612601_LV3TVq did work. It started playing a slideshow (jpeg preview image instead of playing the video). But http://smugmug.com/Tours/20110615-4116-S-Atlanta-Ave/17612601_LV3TVq#!i=1344454513&amp;k=PQSXrW6 embedded the video only and was able to play it.

Anyways, sorry for posting the 2 q's in one (that one probably should have been in support forums).

Here's why I posted here... talking about the additional parameters... such as 'nologo' in the URL, like the iframe embed code can do. Is that something that can already be done or that will be able to be done once there's an update?

Note: See TracTickets for help on using tickets.