Change hook 'post_edit_form_tag' from an action to a filter

[mikeschinkel]

Currently the file /wp-admin/edit-form-advanced.php uses a hook to allow for a plugin to add attributes to the <form> tag via the 'post_edit_form_tag' action. The primary motivation was to allow a plugin to add enctype="multipart/form-data" as per ticket #10518.

Unfortunately using an action can potentially result in multiple plugins emitting duplicated attributes which, while not harmful because of (most?) browser resilience is sloppy, and it does not allow one plugin to override another without reverting to sloppy fiddling with hook internals.

Changing it from an action to a filter as per the attached patch is backwards compatible with existing plugins but will allow future plugins to be more savvy about how they approach adding an attribute to the <form> element.

[mikeschinkel]

Change hook 'post_edit_form_tag' from an action to a filter

[dd32]

is backwards compatible with existing plugins

Not really. Any functions hooked as an action would return null, if they return after a filter hooked function, the value of the filter is effectivly ignored, correct?

[nacin]

Yeah, an action here is kind of messy, but it was consistent with other areas. As dd32 said, attached actions will nuke filters, and then it's a priorities game. You don't need to muck with internals. One could do remove_all_actions( 'post_edit_form_tag' ), no?

[mikeschinkel]

Replying to dd32:

is backwards compatible with existing plugins

Not really. Any functions hooked as an action would return null,

If the action returns null then the echo echos nothing. An action won't be there unless is actually does the echo. So it is 100% backwards compatible because it will behave exactly like it does with the action.

if they return after a filter hooked function, the value of the filter is effectivly ignored, correct?

But 1.) that does make it not backward compatible because today nobody is using filters for this (unless they are screwing up), and 2.) for future filters added then your concern is only relevant if someone writes a new filter that gets nullifying by a later priority action, but that is 100% possible with filters already if the later filter decides to eat the value. So just document in the code that filters may get overridden by legacy hooks. Better than the current case of outputting invalid HTML.

Yeah, an action here is kind of messy, but it was consistent with other areas. As dd32 said, attached actions will nuke filters, and then it's a priorities game.

But filters can nuke filter so I don't see how this is a justification for leaving it as is.

One could do remove_all_actions( 'post_edit_form_tag' ), no?

Only if you know what actions to remove and which ones not to remove. Who knows what some plugin is doing as a side-effect in their 'post_edit_form_tag'? All you alternatives seem a lot worse than just putting the burden on the future writers of filters to use a higher priority.

[dd32]

If the action returns null then the echo echos nothing.

Unless the filter has both an action AND a filter attached.
Eg:

add_filter('filter', function($s) { return $s . '1'; });
add_action('filter', function() { echo '2'; });
add_filter('filter', function($s) { return $s . '3'; });

echo apply_filters('filter', '');

echo's '23', the 2 from the action, and 3 from the 3rd filter, the 1 from the first filter has been null'ed as the return value of the action is NULL and has overridden the 1.

[nacin]

Replying to mikeschinkel:

Only if you know what actions to remove and which ones not to remove. Who knows what some plugin is doing as a side-effect in their 'post_edit_form_tag'?

Don't see the difference between the status quo -- needing to remove everything -- than the proposed change, which not only will require parsing whatever earlier filters have added, but also what back-compat actions have been added, as well as priorities because later actions will override things (in a much different way than filter followed by filter would). I prefer this as an action. Not a filter.