#14881 closed defect (bug) (wontfix)
do_action should not pass empty string by default
Reported by: | nacin | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Plugins | Keywords: | has-patch |
Focuses: | Cc: |
Description (last modified by )
function test( $a = true ) { var_dump( $a ); } add_action( 'test', 'test' ); add_action( 'test', 'test', 10, 0 ); do_action( 'test' );
Second example is bool(true) as expected, but the first is a zero-length string.
In this case, since do_action() is not passing any arguments, then no arguments should be passed to test() -- even if by default, one argument gets passed.
The issue here is how do_action() is defined: function do_action( $tag, $arg = '' );
. There's our zero-length string. Of note, apply_filters() does not have this same issue.
The solution looks something like what I have attached. Has yet to be benchmarked.
Of note, this does not appear to have any direct implications for passing all args by default, as proposed in #14671, but I wanted to reference it anyway.
Attachments (1)
Change History (15)
#3
@
14 years ago
Related: #14671 - Deprecate the "accepted args" argument in add_filter() and add_action()
#7
@
9 years ago
Just encountered this gotcha last week.. What's the holdup? Are there plugins that depend on this bug?
#8
@
9 years ago
Based off of your diff it seems that you are returning an empty array instead of an empty string. Would you want to return NULL instead? An empty array might confuse developers into thinking that it is actually an argument to the action. However, a NULL value is much more related to an error. Please let me know your thoughts @nacin.
#10
@
8 years ago
will this ever be solved?
for example,we have:
add_action('wp_head','my_function');
function my_function($first=false){
var_dump($first); exit;
}
it doesnt show false, instead something empty string is passed from the action... why? I think actions should not touch the arguments of functions..
#11
@
7 years ago
The bug is still valid and there is a patch by @nacin to fix it.
Any objections on doing so?
#12
@
7 years ago
I'm not sure we can actually change this.
there's bound to be plugins which rely upon this behaviour, hooking into multiple things, etc.
#13
@
7 years ago
- Keywords 3.2-early removed
- Milestone Future Release deleted
- Resolution set to wontfix
- Status changed from new to closed
It would be nice to fix this, but this ship sailed alongside #14671.
#14
@
4 years ago
I just want to note that this bug has serious implications if your callback has a type declaration other than string
for the first argument. You may argue that using type declarations in hook callbacks is counterproductive, but I don't know. We've had type hinting for classes in PHP for nearly 16 years, and type declarations for classes and built-in types for over 4 years. There are some of us who feel it's silly to not be able to use type declarations everywhere that's possible because of one stupid WordPress bug that's been around forever. Like, every time I write a callback I have to go through a mental checklist: "Is this for an action? Will it be called with just do_action()
(as opposed to do_action_ref_array()
)? Do I want the first param to be optional? Do I want it to be a type other than string? If all of the above, don't put a type declaration."
Please take not of this ticket as well: #14789