Calling bp_core_new_nav_item() without screen_function parameter generates PHP warning

[doublesharp]

Adding a new navigation item via bp_core_new_nav_item() without passing screen_function as a parameter (if it uses a default subnav screen for example) will generate the following warning when do_action('bp_screens'); is called:

[29-Jan-2013 23:21:55 UTC] PHP Warning:  call_user_func_array() expects parameter 1 to be a valid callback, no array or string given in /Workspaces/PHP/wordpress/wp-includes/plugin.php on line 406

Changing the default value for this parameter in bp-core/bp-core-buddybar.php from false to '__return_false' uses a valid callback and does not generate the warning.

The same fix can be applied to bp_core_new_subnav_item(), bp_core_new_nav_item(), and bp_core_new_nav_default() asin the attached patch.

[doublesharp]

use 'return_false' instead of false for default screen actions

[r-a-y]

Sounds valid. Moving to 1.7 milestone for testing.

[DJPaul]

The patch works well for me and I'm tempted to commit it, but I think doing this properly would involve method_exists and function_exists checks before make the add_action calls.

[r-a-y]

Switch out is_object() for screen function checks and use is_callable()

[r-a-y]

02.patch is another approach to this issue based on Paul's feedback.

I like this better as:

• We don't have to explicitly set the default screen function to __return_false()
• We can check the existence of the screen function with is_callable() for both methods and functions.

Tested lightly with default BP components and various plugins like BP Docs, BP Followers, bbPress, as well as with a new nav item with no screen function - no errors.

Feel free to commit doublesharp's patch and punt my one to 1.8 if not enough time to test.

[DJPaul]

Thanks for the patch r-a-y, but it's missing most of the is_object logic that's in the current version.

[r-a-y]

Thanks for the patch r-a-y, but it's missing most of the is_object logic that's in the current version.

Unless I'm mistaken, is_callable() already checks for objects or class methods.

See the example on the PHP doc page:
​http://www.php.net/manual/en/function.is-callable.php

[boonebgorges]

In 7171:

Use is_callable() to validate screen_function callbacks before hooking in bp-core-buddybar.php

We were previously using is_object() and other custom verification techniques
to hook screen functions in bp_core_new_nav_item() and elsewhere. These
techniques are limited in that they allow certain sorts of uncallable strings
to pass through. We also should not be reinventing the wheel.

Fixes #4794

Props r-a-y