Make WordPress Core

Opened 14 years ago

Closed 13 years ago

#13817 closed enhancement (fixed)

Reusing the Media Upload kit in a plugin

Reported by: simonwheatley's profile simonwheatley Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.0
Component: Media Keywords: has-patch dev-feedback
Focuses: Cc:

Description

I'm keen to reuse the Media Upload overlay (i.e. the thickboxed iframe from the edit post/page screens) when I need to upload files for a plugin. It seems crazy to be reimplementing file upload and image resizing code when it's built into WordPress core. However the hooks aren't there currently.

The attached proof of concept plugin "rmu-poc.php" shows that it is possible to write a plugin which pops up the upload overlay (install it and go to the edit category screen), but a number of issues prevent it being useful:

The form is setup to be called in the context of the edit post/page screen, for example the JS callback expects to find a function send_to_editor which will accept it's HTML.

It would be handy to insert some additional post fields to be sent when the SWF Uploader uploads files, so the plugin can detect that it should be doing something with that upload.

The get_media_item function needs a way to customise the button label. Currently the proof of concept does a very ugly hack on a translation filter.

Before I get over excited, is anyone up for these changes? If I started cutting some patches, would they be considered?

Attachments (2)

rmu-poc.php (8.5 KB) - added by simonwheatley 13 years ago.
Proof of concept plugin - now with more descriptive description
media.php.diff (2.4 KB) - added by simonwheatley 13 years ago.
Three proposed new filters for media.php - patch refreshed to current trunk

Download all attachments as: .zip

Change History (26)

#1 @simonwheatley
14 years ago

  • Cc simon@… added

#2 @Denis-de-Bernardy
14 years ago

  • Cc Denis-de-Bernardy added

+1

#3 @unsalkorkmaz
14 years ago

  • Cc unsalkorkmaz added

#4 follow-up: @nacin
14 years ago

Fine idea in theory, but we're planning to entirely rewrite media/upload in a future release, which would probably include dropping thickbox in place of an inline uploader, etc... And so I think we would be disinclined to create more points for plugins to hook into a system when we'd break them in the next two major versions.

I would think that given our current development philosophy, nothing we'd build wouldn't be extendable the way you'd want it.

#5 in reply to: ↑ 4 ; follow-up: @filosofo
14 years ago

Replying to nacin:

Fine idea in theory, but we're planning to entirely rewrite media/upload in a future release, which would probably include dropping thickbox in place of an inline uploader, etc... And so I think we would be disinclined to create more points for plugins to hook into a system when we'd break them in the next two major versions.

I would think that given our current development philosophy, nothing we'd build wouldn't be extendable the way you'd want it.

Well, this is a snippet from an IRC conversion on October 17, 2008, the first mention of media being handled in the next version of WordPress (that grep finds for me; public logs don't seem to go back that far).

Oct 17 03:34:56 <DD32>  Ah cool, At least there'll be something left over for 2.8
Oct 17 03:35:15 <janeforshort>  media, media, media and fixing widget management, plus probably inbox

And since there's talk now of using 3.1 to focus on WordPress.org, qua the website not the app, that would put the great media revolution off at least until 3.2, which if history is a guide will arrive a year from now.

All that to say, let's not trade concrete suggestions for vaporware.

#6 @johnbillion
14 years ago

If the media system does get an overhaul and moves to something inline rather than modal, there's still a case for what Simon is suggesting. A system allowing plugins to easily provide a modal media browser/uploader would be great.

#7 @scribu
14 years ago

  • Cc scribu@… added

#8 follow-up: @nacin
14 years ago

Replying to johnbillion:

If the media system does get an overhaul and moves to something inline rather than modal, there's still a case for what Simon is suggesting. A system allowing plugins to easily provide a modal media browser/uploader would be great.

Correct. I hinted to this in the last paragraph of my comment.

Replying to filosofo:

And since there's talk now of using 3.1 to focus on WordPress.org, qua the website not the app, that would put the great media revolution off at least until 3.2, which if history is a guide will arrive a year from now.

I would place money on the suggestion that the next major release focusing on feature development (whenever it is) will include a media/upload focus.

#9 in reply to: ↑ 8 ; follow-up: @filosofo
14 years ago

Replying to nacin:

I would place money on the suggestion that the next major release focusing on feature development (whenever it is) will include a media/upload focus.

Sure, but that's something that will appear in 2011. Let's fix the problems we have, and in the future we can deprecate for the new-and-improved system.

As a wise man once wrote,

It’s called iteration. In an open source project, iteration happens very publicly. ... It’s called a patch for a reason. As we evolved the custom post type system, tons of code got rewritten, some of it dating three years and some of it three days. As we merged in WordPress MU, thousands of lines of code written by Donncha and contributors to MU were in some cases entirely rewritten or simply deprecated and removed. This is how a project evolves, grows, and strengthens.

#10 in reply to: ↑ 9 @nacin
14 years ago

Replying to filosofo:

Replying to nacin:
Sure, but that's something that will appear in 2011. Let's fix the problems we have, and in the future we can deprecate for the new-and-improved system.

As a wise man once wrote,

Touche. :-) Though I would be weary of introducing hooks that we'd then need to support (or, drop, after only one version, if we even release 3.1 versus just stretch the cycle) through what I'd hope to be a complete change to how media works in terms of both UI and API.

#11 @simonwheatley
14 years ago

So... can I push the case for a patch with a light touch, which would enable us to use the media upload kit elsewhere? I've extended my test code (see amended attached proof of concept plugin) and patched the current trunk in as few places as possible to get it working... two proposed new filters to be exact (see attached diff).

The two proposed new filters are:

swfupload_post_params - This allows you to add to or alter the post fields which SWFUpload posts to the server along with the file. Guessing at the future, I'd say this filter would be reusable on the new system Nacin mentions unless we don't end up keeping SWFUpload as the file upload provision.

swfupload_success_handler - This allows you to filter the name of the JS function passed to SWFUpload, which it calls on successful upload. Again, if SWFUpload remains a provision for uploading files then I'd argue that amending this function name would be necessary in the future system.

How does that sound?

#12 @simonwheatley
14 years ago

OK. Now I look at the diff I see three new filters, sorry for the confusion, the third being:

get_media_item_args - This allows you to filter the arguments passed to get_media_item, and I propose using this to set "send" to true so that the action button (named "Insert into Post" when uploading images for posts) is not supressed.

#13 @simonwheatley
14 years ago

OK. Now we're looking at 3.1 in earnest I want to push the case for a light touch patch here... just three filters, what do people say?

(I'm aware I'm pushing it, particularly with Jane's comments on Process and Scope.)

#14 @scribu
14 years ago

  • Keywords has-patch added
  • Milestone changed from Future Release to 3.1
  • Type changed from feature request to enhancement

Looks good to me. Hooks are generally welcome, regardless of scope.

#15 in reply to: ↑ 5 @mikeschinkel
14 years ago

+1 on adding to 3.1.

#16 @mikeschinkel
14 years ago

  • Cc mikeschinkel@… added

#17 @williamsba1
14 years ago

  • Cc brad@… added

#18 @hakre
13 years ago

  • Cc hanskrentel@… added

+1 for everything that brings this forward. maybe there is some json function artifact in core that is helpful for your patch. *maybe*

#19 @sorich87
13 years ago

  • Cc sorich87@… added

#20 @he8us
13 years ago

  • Cc he8us added

#21 @jane
13 years ago

I don't have any objection to adding hooks, and since this patch was updated 2 months ago, if it's sound I'd like to get it in if it can go in today (entering beta).

@simonwheatley
13 years ago

Proof of concept plugin - now with more descriptive description

@simonwheatley
13 years ago

Three proposed new filters for media.php - patch refreshed to current trunk

#22 @simonwheatley
13 years ago

Nearly missed Jane's coment...

Patch refreshed to current trunk, no changes apart from line numbers and some context. Everything still works. Added some description to the proof of concept plugin.

Can we get this in before beta? Please? :)

#23 @nacin
13 years ago

Don't see much of a problem with this.

#24 @ryan
13 years ago

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

(In [16476]) get_media_item_args, swfupload_post_params, and swfupload_success_handler filters. Props simonwheatley. fixes #13817

Note: See TracTickets for help on using tickets.