Merge media buttons into one

[mitchoyoshitaka]

Here's a patch with the filter name 'media_buttons_media_types'.

[scribu]

Pretty sure there's already a filter for this somewhere in there.

[nacin]

Nope. The whole thing is attached to a hook, but that's about it.

You could wedge something into a filter off the mu_media_buttons site option (if multisite) or media_buttons_context (a filter for a string), but that's quite lame.

There should be a filter on the array.

[mitchoyoshitaka]

Indeed, I was just about to filter on the HTML string via 'media_buttons_context' but decided to write a bug and complain about it first. :)

[azaozz]

I'm unsure why there are four buttons there that do the same thing. Perhaps they were needed when this was introduced but have lost their meaning since. It seems confusing that you can upload anything by using any of these buttons. Perhaps when we change the uploader in 3.3 we will switch to using only one "Insert/Upload" button.

Looking at the patch: we already have most of the logic there, don't see a need to repeat it (also really dislike using extract() especially without EXTR_IF_EXISTS / EXTR_SKIP). So this should probably be:

$media_buttons = array('image' => true, 'audio' => true, 'video' => true);
if ( is_multisite() )
  $media_buttons = get_site_option( 'mu_media_buttons' );

$media_buttons = apply_filters( 'media_buttons_media_types', $media_buttons );

$out = '';
if ( ! empty($media_buttons['image']) ) 
  $out .= _media_button(__('Add an Image'), ....

[scribu]

+1 on having a single upload button.

Next to it, we could have the "Insert link" button, as discussed in another ticket.

[patrickgarman]

Another +1 for a single media upload button. Small issue, but it could cause minor confusing even though it will not hinder anyone's ability to upload content.

While we're at it... in multisite installs network admins have the option of what buttons to show. ​http://screencast.com/t/vlv9CyTANB ... might avoid another bug report by taking out those options once the different buttons are gone for good?

[ocean90]

Ticket about the new file uploader #18206

[nacin]

Sadly, we've yet to merge these buttons. Let's do that in 3.4.

[nacin]

Resurrecting this after an IRC discussion, re-classifying it as a UI bug, as it confuses people left and right and comes up in testing all of the time.

I have a patch on the way.

[nacin]

Attached patch:

• Stubs media_buttons(). We are now using the generic 'media' button (which is type = 'file'). I switched to the video image for now as it looks better. Per IRC, we should steal the image from the Media Library screen/menu icon.

• Removes media_upload_audio(), media_upload_video(), and media_upload_file(). These can be stubbed to wrap media_upload_image().

• Removes type_url_form_video(), type_url_form_audio(), and type_url_form_file(). These can be stubbed to wrap type_url_form_image().

• media_upload_type_url_form(), which is the "Insert from URL" form, is the only thing that actually needed any changes. Here, we now have radio buttons that default to "Image" insertion, and allow for "Audio, Video, or Files." Dead-simple jQuery toggles which fields to show. This dates back to crazyhorse, according to jane.

• Replaces the callbacks for the media_upload_audio, media_upload_video, and media_upload_file hooks to be media_upload_image(), to ensure some compatibility when type=audio, type=video, or type=file is requested. (It will take basically no work to also ensure that such links will result in the image fields being hidden by default.)

• Removes the old add_action( 'async_upload_*' ) calls, which are from pre-plupload.

• Removes mu_media_buttons from network settings and code.

Probably needs some tweaking based on other things we find, but it's pretty solid as-is, and handles the merger quite elegantly indeed. I will say though, my original diff will never see the light of day. It was much easier the second time around.

[nacin]

We'll need to do a quick headcount of filters to make sure too much isn't trapped in the rubble.

[SergeyBiryukov]

The patch removes audio_send_to_editor_url, video_send_to_editor_url, file_send_to_editor_url filters.

[nacin]

17578.3.diff​ is the result of a closer inspection of the guts of what we're working with here.

• Merges in code from media_upload_file() (and audio, and video -- these are all the same) to properly put together a simple hyperlink to then send to the editor.

• Restores the three filters that I previously needed to remove. It triggers *_send_to_editor_url with audio, video, or file by checking the file extension (file being the fallback).

• Enables old links to tab=type_url&tab=video (or audio, or file) to still work, by making the audio/video/file form visible and the image form hidden.

• Only do a check on whether the image URL is valid for images. (This is where the green checkmark comes from. We don't do this for audio/video/file.)

• Channeled my inner azaozz and changed how toggle is implemented, moving it to a single class on a single parent element, with CSS to then handle on/off.

• Killed a filter, 'type_url_form_$type', which filtered the entirety of the URL form previously. This is silly. While we can now predict what the type will be (it'll always be image, unless someone does a custom URL, as stated above), the form structure has changed as elements have been added. In return, I've introduced type_url_form_media, a generic filter people can now use.

• Restores the async_upload_* hooks as they're still needed for async-upload.php (though I've put in a Q to azaozz as to whether we need to keep that file around).

At this point, it's been pretty well hammered by both myself and ryan.

[nacin]

Only things left really are to study the restructuring of functions.

• A number of functions have been removed. These can become deprecated wrappers.

• The remaining functions, like type_url_form_image() and media_upload_image(), should probably be renamed, and turned into deprecated wrappers.

• We should grep through the plugins directory and see how plugins are using the various functions and filters at play here.

Also, Ryan suggested we refactor mu_media_buttons into a single checkbox for enabling/disabling uploads, as that's what people use it for. (Even thought it doesn't do that currently, and never blocked anything.) Seems fine to me, though we'll need to probably hook in a bit deeper in spots (like actually restricting things from being uploaded).

[nacin]

The more I look, the more completely dead code I find in wp-admin/includes/media.php. Some of it rendered useless by Plupload, others over the years.

media_upload_image(), in the patch, currently handles all "Insert into Post" clicks for video/audio/files and images. Only, the image handling code is dead. It's unreachable. The form will never submit data back for images. It does it via JS. The image_send_to_editor_url hook is thus completely dead.

I found this while doing a full audit of all hooks, using the 'all' hook. And I did find we're in good shape here.

Doing a commit on this, minus the changes for mu_media_buttons (other than that they won't hide buttons). Not dealing with function renaming or deprecation yet. Will follow up with any breakage in the morning. Have yet to grep the plugins repo -- a new slurp is currently in progress. Doing the commit mainly so Jane can have it during testing.

[nacin]

In [18831]:

Merge the four media buttons into one.

• Lots of red.
• Removes the type_url_form_\$type filter, where \$type is audio, video, file, or image.
• Replacement filter is a generic type_url_form_media filter for the new unified form.
• Some functions have gone missing; they'll be restored and deprecated in a later pass.

see #17578.

[nacin]

In [18832]:

Compress and bump media.css. see #17578.

[azaozz]

auto-detect-img.patch changes the JS so it shows the image options when the user inserts an URL pointing to image. It removes the radios for selecting type.

[jane]

I thought we were going to use the Media icon from the left nav menu: camera + musical note. Using the video icon makes it more confusing. Can that be replaced, please?

[nacin]

Replying to jane:

I thought we were going to use the Media icon from the left nav menu: camera + musical note. Using the video icon makes it more confusing. Can that be replaced, please?

Yep, will handle it on the next pass.

[jane]

Let's get this icon swapped out so we can close this ticket before calling beta 1.

[nacin]

In [18890]:

UI touches for the single media button. see #17578.

[nacin]

In [18892]:

Kill the multisite mu_media_buttons option. see #17578.

[nacin]

In [18893]:

Oops. see #17578.

[nacin]

In [18894]:

type_url_form_image() becomes wp_media_insert_url_form(), media_upload_image() becomes wp_media_upload_handler(). type_url_form_* and media_upload_* are restored as wrappers. Don't deprecate yet. see #17578.

[jane]

Looking good. Can we drop the "or" to be small and on line with Select Files button, and put cancel a p space below the select?

[ocean90]

Can we drop the "or" to be small and on line with Select Files button, and put cancel a p space below the select?

Done in 17578.break-line.patch​. See ​http://cl.ly/Ajg7. But I think this could be difficult to translate for some languages.

[SergeyBiryukov]

Replying to ocean90:

But I think this could be difficult to translate for some languages.

With a placeholder in the string (17578.break-line.2.patch​), should be fine, I guess.

[ocean90]

should be fine, I guess.

Should be, but breaks if there is a language which needs or on the first place: {or} {option1}, {option2}.
Is there a language like this? I don't know. We can add a second placeholder, but then translators can/will change the order of the items. Is that okay?

[SergeyBiryukov]

Ah, I see your point now. They'd probably have to work around this in some way.

"Add New" isn't very flexible in that sense too: comment:ticket:16406:20.

[azaozz]

In [18918]:

Fix prepending $editor_id to the Add Media link, see #17578

[azaozz]

We should deprecate _media_button($title, $icon, $type, $id) as it's not used any more. Or should we just delete it as a punishment to plugins that use "private" functions? (joking)

[nacin]

I hadn't yet deprecated anything since I was waiting to un-deprecate the stuff in #18785 and didn't want a straight merge -c to fail. I'll sweep through, there are a number of remaining tweaks I'd like to make.

Thanks for fixing the $editor_id. Thought I committed that.

[nacin]

Replying to ocean90:

Is there a language like this? I don't know. We can add a second placeholder, but then translators can/will change the order of the items. Is that okay?

We can just do %s or %s. That forces the upload to always come second and I think it always works, even RTL.

[nacin]

Can someone please spin off the patches for the upload text into another ticket?

[nacin]

In [18992]:

Fix media/audio/file URL insertion. Strip slashes off title text. Leave red X to images only. fixes #14996. fixes #17578.

[SergeyBiryukov]

Replying to nacin:

Can someone please spin off the patches for the upload text into another ticket?

#18994

[nacin]

In [19046]:

Deprecate media_upload_(image|audio|video|file)(), type_url_form_(image|audio|video|file)(). These now wrap wp_media_upload_handler() and wp_media_insert_url_form(). see #17578.