Backwards compatibility for admin/includes/media.php hooks

[nacin]

Lots of hooks in this file. Probably the most common one is attachment_fields_to_edit, but there are others.

These need to be implemented in either the new post.php screen (#21391) or the media modal (#21390) depending.

I've been making mental notes, but what this really needs is a full audit of what we need to make sure is still backwards compatible, then keep it functional (in deprecated form, most likely).

koopersmith is already working on making media tabs back compat; that's a component of this too.

[DrewAPicture]

It would be nice to maintain the post-upload-ui action in the modal somewhere.

Though I'm sure there are plenty of other plugins that hook into post-upload-ui our use case is related to my previous query about taking advantage of plupload's built-in scaling functionality. In 3.4.2, we hook to post-upload-ui which in effect allows us to set the height/width js vars that plupload needs to allow max-size scaling on upload.

[Chouby]

Glad to see this ticket open. For me the most important part is the backward compatibility in the media modal as I still do not foresee how it will work.

I have an additionnal issue and I don't know if this ticket is the right place to ask for it as it is not a hook, though it concerns backward compatibility for the media modal:

In 3.4 and older, when we filter the query done to display media in the library tab of the 'add media' form, it is possible to know the current edited post through $_REQUEST['post_id']. In 3.5 beta 2, $_REQUEST does contain only 'action' and 'query' indexes. Could it be possible to set $_REQUEST['post_id']?

[koopersmith]

In 22523:

Media: Backwards compatibility for media_upload_tabs.

• Adds createIframeStates() to the MediaFrame view. It creates states and bindings for the media_upload_tabs output, and is included on MediaFrame.Post by default.
• Hijacks tb_remove() when the media modal is open to ensure the modal closes correctly.
• Adds a chromeless parameter to thickbox media tab URLs to render the UI without the old row of tabs.

see #22186, #21390.

[nacin]

In 22524:

Don't unnecessarily translate media upload tab strings. see #22186.

[nacin]

22186.diff​ is an attempt at providing a framework for making attachment_fields_to_edit and _save backwards compatible in some way. Here's some random testing code. Fire this up in 3.4 to see it in action when editing a media item from the Library.

add_filter( 'attachment_fields_to_edit', function( $fields, $post ) {
	$fields['nacin_menu_order']['label'] = 'Nacin Order (cannot be 42)';
	$fields['nacin_menu_order']['value'] = $post->menu_order;
	return $fields;
}, 10, 2 );

add_filter( 'attachment_fields_to_save', function( $post, $attachment ) {
	if ( isset( $attachment['nacin_menu_order'] ) ) {
		if ( $attachment['nacin_menu_order'] == '42' )
			$post['errors']['nacin_menu_order']['errors'][] = 'Nacin order cannot be 42.';
		else
			$post['menu_order'] = $attachment['nacin_menu_order'];
	}
	return $post;
}, 10, 2 );

[nacin]

Test code for core tabs.

[koopersmith]

In 22541:

Media: Add backwards compatibility for attachment_fields_to_edit and attachment_fields_to_save. see #22186.

[nacin]

Here's the results of an audit of all hooks in wp-admin/includes/media.php. The big stuff is already handled. Some of these can simply fall by the wayside, while others might deserve a shim.

A lot of admin/includes/media.php was rendered in some way obsolete in 3.3 when the media buttons were merged into one (#17578) and Plupload was introduced.

Filters tied to the old dialog:

• media_upload_tabs — handled
• media_upload_default_tab — could be supported if we wanted, but isn't really relevant anymore, and appears unused by plugins
• media_buttons_context — no longer relevant, this was the "Upload/Insert" label
• {$type}_upload_iframe_src — no longer relevant
• media_upload_form_url — bunch of filters on "media-upload.php"
• get_media_item_args — could be used to decide whether certain things were displayed for an item, including "Insert into Post" and a "Delete" link. We can support these via get_compat_media_markup()
• type_url_form_media — chunk of html for "Embed from URL", no need to support
• media_upload_mime_type_links — mime-based filter links, which we could support

Filters for attachment data:

• image_size_names_choose
• attachment_fields_to_save — handled
• attachment_fields_to_edit — handled
• media_meta — handled

Filters for sending to the editor:

• {$type}_send_to_editor_url (type = file, audio, or video) — used when inserting a non-image from an external URL
• image_send_to_editor_url — this is nearly always bypassed already, thanks to addExtImage in JS

• disable_captions — we should support this. Note, we only check it for post insertion itself: for whether it reaches the editor, and whether the field is shown for "from URL" insertion for images. The caption field is not otherwise hidden in the media library

Filters for "Insert into Post":

• media_send_to_editor — fires for anything inserted into the post
• image_send_to_editor — fires for any images inserted into the post (get_image_send_to_editor() is hooked into media_send_to_editor, via image_media_send_to_editor())
• image_add_caption_shortcode — adds the caption shortcode for any images inserted into the post (inside image_add_caption(), which is hooked into image_send_to_editor)

Upload UI filters:

• upload_post_params (new in 3.3) — pretty much inescapably tied to the old uploader, not much we can do here unless we wanted to support routing to old-style tabs
• plupload_init (new in 3.3) — the plupload_default_settings hook seems sufficient. array_merging these seems like it would cause more harm than good

Upload UI actions:

• upload_ui_over_quota (new in 3.5)
• pre-upload-ui
• pre-plupload-upload-ui (new in 3.3)
• post-plupload-upload-ui (new in 3.3, used to display "use the browser uploader?")
• pre-html-upload-ui
• post-html-upload-ui (used to display "use the multi-file uploader?")
• post-upload-ui (used to display "you can add titles and descriptions", which is useless)

[koopersmith]

In 22556:

Media: Improve backwards compatibility for image_size_names_choose filter. see #22186, #21390.

[koopersmith]

In 22557:

Media: Add support for applicable *-upload-ui hooks.

Supports pre-upload-ui, pre-plupload-upload-ui, post-plupload-upload-ui, post-upload-ui.

see #22186, #21390.

[trepmal]

It used to be possible to put an image inside the custom media tabs, but with the new modal, the <img> is converted to entities

Secondly, after inserting an image from a custom tab, clicking Add Media tries to return to the same tab - but it's empty.
That was awkward, here's a screencast: ​http://screencast.com/t/6sVsCfB6R1

And my test plugin: ​http://kdl.dropmark.com/25665/1224781

[Chouby]

Here are some comparison results between WordPress 3.4.2 and WordPress 3.5-beta3-22613.

For the "Edit Media" panel:

• In WP 3.4, the filter attachment_fields_to_edit filter is called
  • $pagenow is set to 'media.php'
  • $screen->base is set to 'media'
• In WP 3.5, the filter is not called

For the "library" tab of the "Add media" modal (only the new one for WP 3.5):

• in WP 3.4, the filter parse_query is called
  • $pagenow is set to 'media-upload.php'
  • $screen->base is set to 'media-upload'
  • $_REQUEST['post_id'] refers to post currently edited
• In WP 3.5, the filter parse_query is called but
  • $pagenow is set to 'admin-ajax.php'
  • $screen->base is not set
  • $_REQUEST['post_id'] is not set
• In both cases, the attachment_fields_to_edit filter is called for each picture in the library

I will wait for a future nightly build to go on my tests (currently the new modal is broken when WP_DEBUG is set to true)

[koopersmith]

Thanks for the comparison, Chouby.

Replying to Chouby:

I will wait for a future nightly build to go on my tests (currently the new modal is broken when WP_DEBUG is set to true)

Adding define( 'SCRIPT_DEBUG', true ); to wp-config.php will help — it makes sure you're using the development scripts instead of the minified scripts, which often lag behind.

[nacin]

In 22670:

Avoid doing all of the extra work in get_attachment_fields_to_edit() for fields we will just discard. We just need the filter, and the taxonomies. see #22186.

[nacin]

In 22673:

Move code from image_attachment_fields_to_edit() to get_attachment_fields_to_edit(). The level of abstraction is unnecessary, and it makes it more difficult to call only the filter in the new media modal. Also, that function is sloooow. see #22186.

[nacin]

In 22674:

Call wp_cache_set() with a group and then an expiration in wp_mime_type_icon(). fixes #22503. see #22186.

[nacin]

In 22679:

Remove errant return. props SergeyBiryukov. see #22186.

[nacin]

Back compat handlers for sending to the editor.

[koopersmith]

In 22761:

Media: Backwards compatibility for the disable_captions filter. see #22186, #21390.

[koopersmith]

In 22768:

Media: Backwards compatibility for the many send_to_editor filters. props nacin. see #22186, #21390.

[nacin]

In 22778:

Remove legacy media uploader bits from custom-header.php and custom-background.php. see #22186, #21390.

[nacin]

In 22783:

Support attachment_fields_to_save and attachment_fields_to_edit for attachments going through post.php. see #22186. see #21391.

[nacin]

Based on the summary above, we still need to review:

• get_media_item_args
• Upload UI actions

I'm not convinced on the feasibility or wisdom of supporting get_media_item_args, and upload UI actions are there; they just might get touched up. So, restoring this ticket's priority to normal.

[Chouby]

Replying to koopersmith:

Adding define( 'SCRIPT_DEBUG', true ); to wp-config.php will help — it makes sure you're using the development scripts instead of the minified scripts, which often lag behind.

Yes. SCRIPT_DEBUG is set to true. But it's all my fault. Setting WP_DISPLAY_DEBUG to false solved the issue caused by a notice:
PHP Notice: Undefined offset: 1 in /wp-includes/media.php on line 1242 (build beta3-22734)
This notice was due to an attachment created without media by my plugin which currently does not work correctly with the current beta version of WP 3.5

[nacin]

Replying to Chouby:

PHP Notice: Undefined offset: 1 in /wp-includes/media.php on line 1242 (build beta3-22734)
This notice was due to an attachment created without media by my plugin which currently does not work correctly with the current beta version of WP 3.5

That sounds like a valid bug, can you report it? In 3.4 we never made the explicit assumption that a mime type has a minimum of two parts when you split it by "/". In 3.5, we do.

[Chouby]

Done. See #22532

[koopersmith]

In 22818:

Media: Display WordPress and plupload error messages whenever an upload fails. see #22243, #22186, #21390.

[koopersmith]

In 22819:

Media: Make the pre-upload-ui hooks share an element with the post-upload-ui hooks. see #22186, #21390.

[koopersmith]

In 22821:

Media: Make friends with media_upload_form. Adds notices for browser incompatibility, upload limits, maximum file size, and large file issues to the uploader. fixes #22243, see #22186, #21390.

[Chouby]

I go on with my tests.

In RC1, the filter 'attachment_fields_to_edit' is OK. But unlike in WP 3.4, the attachment taxonomies are not automatically saved anymore. A workaround is to use the filter 'attachment_fields_to_save' as we would do for other custom fields.

I still did not find a way to know (in the filter 'parse_query') which post is currently edited when using the new "Add Media" modal (to replace $_REQUEST['post_id'] which was available in 3.4)

[batmoo]

Is media_buttons_context being deprecated or removed completely? (from [22828])

[nacin]

Replying to batmoo:

Is media_buttons_context being deprecated or removed completely? (from [22828])

It *should* be removed completely, as it was only supposed to be a way to change the label "Upload/Insert". Alas, a lot of code inaccurately used it instead of media_buttons to insert media.

We need to remain compatible here, though it won't be easy. We need some research into the common ways of modifying this string to insert more things. I imagine since you're asking, you've found something?

[nacin]

I've carried media_buttons_context into #22559.

[batmoo]

Replying to nacin:

We need to remain compatible here, though it won't be easy. We need some research into the common ways of modifying this string to insert more things. I imagine since you're asking, you've found something?

Yep, though, being used incorrectly as you note (specially, using it to add media button instead of the media_buttons action). I don't think we need to support compatibility for the incorrect usages unless there's a compelling reason that people are using the context filter over the action (which I don't have).

[Chouby]

in RC1, it seems that the filter 'attachment_fields_to_save' is not called from the new "Add media" modal. In WP 3.4, it was called when clicking on "save all changes" (the button does not exist anymore) and when clicking "insert into post" (this does not work in WP 3.5 RC1)

[Chouby]

Another issue found in RC1 with the new "Add media" modal:
If I directly upload file(s), the attachment details is displayed without fields created by 'attachment_fields_to_edit'
If I first go to the library, the fields are correctly displayed.

[nacin]

Replying to Chouby:

in RC1, it seems that the filter 'attachment_fields_to_save' is not called from the new "Add media" modal. In WP 3.4, it was called when clicking on "save all changes" (the button does not exist anymore) and when clicking "insert into post" (this does not work in WP 3.5 RC1)

It should be fired via Ajax request, as you go. Can you open up an individual ticket for this issue?

[nacin]

Replying to Chouby:

Another issue found in RC1 with the new "Add media" modal:
If I directly upload file(s), the attachment details is displayed without fields created by 'attachment_fields_to_edit'
If I first go to the library, the fields are correctly displayed.

Can you open up an individual ticket for this issue?

[Chouby]

Done. See #22577 and #22578

[nacin]

To address some of Chouby's comments, we should definitely have the XHR requests query-attachments, save-attachments, and save-attachments-compat pass $_REQUEST['post_id'] back, for consistency with the old way.

I'm also okay with a set_current_screen( 'media-upload' ) in each of those, or at least in query-attachments.

Chouby, if there is anything else you have pointed out (or have not yet pointed out), please open fresh tickets reported against trunk. In an ideal world, your plugin works without modification in 3.5 — I'm more than willing to entertain all bug reports that can get us to that.

[nacin]

Replying to nacin:

To address some of Chouby's comments, we should definitely have the XHR requests query-attachments, save-attachments, and save-attachments-compat pass $_REQUEST['post_id'] back, for consistency with the old way.

I'm also okay with a set_current_screen( 'media-upload' ) in each of those, or at least in query-attachments.

I opened #22588 for these things.

[nacin]

An update on the audit:

Filters tied to the old dialog:

• media_upload_tabs — Supported.
• media_upload_default_tab — Not supported. Not really relevant, and appears unused by plugins.
• media_buttons_context — Will be supported, see #22559.
• {$type}_upload_iframe_src — No longer relevant.
• media_upload_form_url — No longer relevant.
• get_media_item_args — Not currently supported.
• type_url_form_media — Not supported.
• media_upload_mime_type_links — Not supported. Chunk of HTML.

Filters for attachment data:

• image_size_names_choose — Supported.
• attachment_fields_to_save — Supported.
• attachment_fields_to_edit — Supported.
• media_meta — Supported.

Filters for sending to the editor:

• {$type}_send_to_editor_url (type = file, audio, or video) — Supported.
• image_send_to_editor_url — Not supported. This filter was already bypassed thanks to addExtImage.

• disable_captions — Supported.

Filters for "Insert into Post":

• media_send_to_editor — Supported.
• image_send_to_editor — Supported.
• image_add_caption_shortcode — Supported.

Upload UI filters:

• upload_post_params (new in 3.3) — Not supported.
• plupload_init (new in 3.3) — Not supported.

Upload UI actions:

• upload_ui_over_quota — Supported.
• pre-upload-ui — Supported.
• pre-plupload-upload-ui — Supported.
• post-plupload-upload-ui — Supported.
• pre-html-upload-ui — No browser uploader in the dialog, but kept on media-new.php.
• post-html-upload-ui — No browser uploader in the dialog, but kept on media-new.php.
• post-upload-ui Supported.

---

End result: Only get_media_item_args is left outstanding, as "Not currently supported". We need to figure out how it works, how it was used by plugins, how it applies to the new workflow, and whether it should still be supported.

[bbrooks]

In the new media UI, custom fields added by hooking into: 'attachment_fields_to_edit' do not get submitted with a nonce, fail ajax verification, and thus do not retain any information entered by the user.

Here's the ajax form data being submitted for the custom fields:

action:save-attachment-compat
id:5555
attachments[5555][name_of_custom_field]:sample custom field text

Compare this to the same submit for the 'alt' field of an image:

action:save-attachment
id:5555
nonce: g5354sd343
changes[alt]:sample alt text

And this is the notice I get when a custom media field attempts to update on blur:

PHP Notice: Undefined index: nonce in [...]/wp-includes/pluggable.php on line 832

[nacin]

Replying to bbrooks:

In the new media UI, custom fields added by hooking into: 'attachment_fields_to_edit' do not get submitted with a nonce, fail ajax verification, and thus do not retain any information entered by the user.

Yep, good catch. Just fixed in #22577.

[nacin]

get_media_item_args is a filter on the arguments passed to get_media_item(), which are:

• 'errors' => null
• 'send' => $current_post_id ? post_type_supports( get_post_type( $current_post_id ), 'editor' ) : true
• 'delete' => true
• 'toggle' => true
• 'show_title' => true

errors is an array of potential errors that get passed from the handler to get_attachment_fields_to_edit(). We don't actually display (or pass along) these errors in the back compat code for 3.5. Hey, at least we handle the fields...

send is a boolean, for whether the item should be given the opportunity to be sent to the editor. This was always simply "true" until 3.0, when it then changed to whether the post type supported the editor, it seems for post-thumbnail purposes. It's also changed quite a bit over the years since being introduced in [6876] (and filtered since [16476]) — see [19350], [15920], [14146], [14118]). There's never been a core reason for "false". Plus, with the new view changes, supporting this really doesn't make much sense.

delete is a way to specify if the delete link should show. It was set to false on media.php, which is now no longer exposed (#21391). We don't have delete links yet (#22524). If we did, this *could* be a reason to support this filter. But I don't think it will be missed.

toggle is whether the panel should be able to be opened and closed (the accordion-style media items). This isn't relevant with the new UI.

show_title was again used only on media.php. Not relevant with the new UI.

[nacin]

A little under 30 instances of get_media_item_args in plugins. All but 1 force send to on. Most of those (all but five) using one of two functions from a plugin options/form framework (the methods are cmb_force_send and allow_img_insertion).

The only one that is using get_media_item_args() for something other than send = true, is for send = false, to implement some kind of post thumbnail functionality. This plugin was recipe-schema. I activated it, and after figuring out how to use it (you need to create a recipe, then add that recipe to a post), I was able to trigger the old thickbox dialog and successfully attach a thumbnail.

So, it seems get_media_item_args shall die with the old dialog.

[nacin]

I know of no other backwards compatibility adjustments required for media in 3.5. New tickets for any new issues, please!

[ZaneMatthew]

Sorry Nacin, found this one, not sure if its an issue along the lines of what your looking for.

The second parameter in the hooks seems not to have the same indexes in that $attachments array.

I have a hook similar to this:

function my_func( $post, $attachment ){
    print '<pre>';
    print_r( $attachment );
    print '</pre>';
    die();
}
add_action('attachment_fields_to_save', 'my_func');

which on WP older than 3.5-rc

I receive this:

Array
(
    [menu_order] => 0
    [post_title] => 
    [image_alt] => 
    [post_excerpt] => 
    [post_content] => 
    [url] => 

)

RC gives me nothing.

[nacin]

Hi Zane, could you open a new ticket for this?

[ocean90]

But before this, be sure that you are using add_filter('attachment_fields_to_save', 'my_func', 10, 2);.

[Chouby]

I just checked with RC2 and confirm. Only custom fields are in $attachment. The same in "edit media" and the new "Add media" modal. So it's worth that Zane opens a new ticket for this.

[ZaneMatthew]

Ticket created here: #22664