broken compat attachment attributes save

[geminorum]

cannot save data via attachment_fields_to_save filter.
probably because wp_ajax_save_attachment_compat() expects $_REQUESTattachments? which not provided by save-attachment-compat js sender.
there are only post_id, id (attachment_id), and the changes.

[dd32]

To make it easier for others to test and debug, can you please post some code which can be used to trigger it, or a plugin name (and version) which can be used to duplicate it?

[geminorum]

​wp.media.model.Attachment.saveCompat sends id, post_id, nonce and the new changes.

​wp_ajax_save_attachment_compat() expects attachments or attachmets id.

so cannot use ​attachment_fields_to_save to save extra data.

it triggers by my theme framework, not published yet.

unfamiliar with new js workflow, and sorry, don't know how to build a patch.

[geminorum]

sample plugin

[geminorum]

the attached works on WP 3.4.2, but not on WP 3.5

[crocro]

I'm experiencing this problem as well : this occurs because of the checkboxes and jQuery's serializeArray function.

Further explanation : when the checkbox goes unchecked, the serializeArray function (at line 4139 of media-views.js) won't take it into account (the function is designed to work like a browser : when the checkbox is unchecked, no info is sent via HTTP POST requests).

As a consequence, when wordpress receives the AJAX request, the post parameters will look like that :

action	save-attachment-compat
id	34872
nonce	a1b5113007
post_id	34865

Whereas it's expecting something like this to work (just like when a classic text input is empty) :

action	save-attachment-compat
id	34872
custom_checkbox_field	(empty)
nonce	a1b5113007
post_id	34865

Therefore, it won't even bother calling "attachment_fields_to_save" filter and returns an error right away because it can't guess which field has been edited.

The best (and cleanest) patch I could imagine of would be changing serializeArray to a function that takes into account empty checkboxes as well.
Sending the "custom_checkbox_field (empty)" additional post parameter will be enough to make it work correctly.

I can work on a patch but I'm not very familiar with Wordpress core code.
However, editing the media-views.js file by adding :

jQuery('input[type=checkbox]:not(:checked)', this.$el).map(function() {
    data[this.name] = '';
});

at line 4201 would be enough to make it work correctly.

Notice for whom may use this patch : when you add a custom field thanks to "attachment_fields_to_edit" filter, and if you use 'input' => 'html' option for your custom field, you've to name it "attachments[$attachment->ID][name_of_your_custom_field]" (where $attachment->ID is the attachment ID and name_of_your_custom_field is a field name) in order for Wordpress to recognize it through the AJAX post request.

Regards

[geminorum]

I think we have no problem with empty checkboxes. if it's not on the POST then it's unchecked.

But the "attachments[$attachment->ID][name_of_your_custom_field]" part works. the problem was me not following the core.

thanks crocro, and sorry to bother.

[crocro]

Yep, but if it's not on the POST, Wordpress will never apply attachment_fields_to_save filter.

It means, when you try to uncheck the checkbox via the new AJAX media interface, there isn't any chance you can save the result to the database or such things. "wp-admin/admin-ajax.php" will answer "{"success":false}" and that's it - attachment_fields_to_save won't be called.

I don't know if you can test this ?

[nacin]

I think in order to account for unchecked checkboxes, the filter should always be applied.

[geminorum]

I've got it working by including a hidden attachments[$attachment->ID] field.

Also, I think it's better to delay the ajax call until the last one responses. It gets weird when you click multiple checkboxes on the same time.

[nacin]

The problem with attachments[$attachment_id] is it must come at the front, and then we must also turn a zero-length string into an empty array in the ajax handler (or in the JS).

22868.diff​ just adds a hidden menu_order field, which was always sent in 3.4 anyway.

[nacin]

In 23290:

Whenever we have compat fields to render, send a dummy menu_order field (which was always sent in 3.4) to ensure an unchecked checkbox can still be processed by attachment_fields_to_save. fixes #22868.

[nacin]

In 23291:

Whenever we have compat fields to render, send a dummy menu_order field (which was always sent in 3.4) to ensure an unchecked checkbox can still be processed by attachment_fields_to_save.

Merges [23290] to the 3.5 branch.
fixes #22868.