Fixed controller_pre_dispatch inconsistency

pegasus

Well-known member
There is a discrepancy between the callback signature as discussed in the tip when creating a listener at controller_pre_dispatch:
Callback signature:

XenForo_Controller $controller, $action
Arguments:

  1. XenForo_Controller $controller - the controller instance. From this, you can inspect the request, response, etc.
  2. string $action - the specific action that will be executed in this controller.
Event Hint: Name of the controller class that is being run.
And the actual implementation of controller_pre_dispatch in library/XenForo/Controller.php:
PHP:
XenForo_CodeEvent::fire('controller_pre_dispatch', array($this, $action, $controllerName), $controllerName);
Here we see that 3 arguments are actually passed, $this (or $controller), $action, and a third one $controllerName, which is really quite handy if we want to pre-do some things only for certain controllers. (Even though $controllerName is the hint too, I don't see that ::fire is popping any arguments that were used as hints before call_user_func_array occurs, so it should still be relevant as a third argument).

However, this discrepancy leads to issues when someone might override the _preDispatch method in their controller, and uses the signature from the admin description, rather than pasting the relevant ::fire line or calling parent::. This has led to different mods using different callback signatures, both on the firing side and on the subscribing side.

I've seen errors caused by this discrepancy (argument 3 missing, et al), and I'm not even positive an add-on is responsible. I haven't searched exhaustively, but please make sure the implemented signature is consistent if there are multiple instances of controller_pre_dispatch in the default package.

You can correct this in one of two ways:
1. Update the listener description so that it mentions the 3rd argument. OR
2. Remove the 3rd argument from the ::fire. Remind coders that there is another way to get the current $controllerName in this context. Alert customers that some mods may be broken by this update.
 
Last edited:
The docs for this have been updated to mention the final argument.

However, there shouldn't be any way for someone to break anything with this. The controller name is passed to the event, but none of the other methods and the only method that receives it is final. There shouldn't ever be any other code triggering that event.
 
Top Bottom