Make WordPress Core

Opened 14 years ago

Closed 7 years ago

Last modified 4 years ago

#14881 closed defect (bug) (wontfix)

do_action should not pass empty string by default

Reported by: nacin's profile nacin Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Plugins Keywords: has-patch
Focuses: Cc:

Description (last modified by nacin)

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)

14881.diff (1.0 KB) - added by nacin 14 years ago.

Download all attachments as: .zip

Change History (15)

@nacin
14 years ago

#1 @nacin
14 years ago

  • Description modified (diff)

#2 @hakre
14 years ago

Please take not of this ticket as well: #14789

#3 @hakre
14 years ago

Related: #14671 - Deprecate the "accepted args" argument in add_filter() and add_action()

#4 @nacin
14 years ago

  • Milestone changed from Awaiting Review to 3.1

#5 @nacin
14 years ago

  • Keywords 3.2-early added
  • Milestone changed from 3.1 to Future Release

#7 @mnelson4
9 years ago

Just encountered this gotcha last week.. What's the holdup? Are there plugins that depend on this bug?

#8 @kjbenk
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.

Last edited 9 years ago by kjbenk (previous) (diff)

#9 @swissspidy
8 years ago

#39867 was marked as a duplicate.

#10 @tazotodua
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 @pavlo.opanasenko
7 years ago

The bug is still valid and there is a patch by @nacin to fix it.
Any objections on doing so?

#12 @dd32
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 @johnbillion
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 @wbeaumo
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."

Note: See TracTickets for help on using tickets.