From: Ori Livneh <ori@wikimedia.org>
Date: Mon, 24 Jun 2013 21:42:20 +0000 (-0700)
Subject: Allow explicit & implicit null returns from hook handlers
X-Git-Tag: 1.31.0-rc.0~19358
X-Git-Url: http://git.cyclocoop.org/%24href?a=commitdiff_plain;h=5ff7e6fb7a36bfcf76b8cd7083fc5ea720db6b52;p=lhc%2Fweb%2Fwiklou.git

Allow explicit & implicit null returns from hook handlers

Most hook handlers are written with the intent of complementing or augmenting
core functionality rather than vetoing it, making it quite natural for a
developer to forget that the caller is waiting for permission to proceed. The
potential for confusion is magnified by the fact that DOM event handlers and
jQuery event handlers are not required to return an explicit value for the
handled event to continue propagating.

This change tolerates null return values (both implicit and explicit -- that
is, both 'return null' and no return statement at all) from hook handlers. To
abort processing, a hook function must return an explicit false or an error
string.

This change should not break any existing hook functions, as returning null is
currently an error.

Bug: 50134
Change-Id: I11deb2117ff9233c77868470f50e0d8f74053545
---

diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 2aee6e4fa0..997dbc6c21 100644
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -256,6 +256,11 @@ changes to languages because of Bugzilla reports.
   will no longer be output and OutputPage::addKeyword no longer exists.
 * Methods Title::userCanEditCssSubpage and Title::userCanEditJsSubpage,
   deprecated since 1.19, have been removed.
+* (bug 50134) Hook functions are no longer required to return a value. When a
+  hook function does not return a value (or when it returns an explicit null),
+  processing continues. To abort the hook, a hook function must return an
+  explicit, boolean false or a string error message. Other falsey values are
+  tantamount to a 'return true' in earlier versions of MediaWiki.
 
 == Compatibility ==
 
diff --git a/includes/Hooks.php b/includes/Hooks.php
index ed8b3edd84..396e360dd8 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -126,6 +126,9 @@ class Hooks {
 	 * @param array $args  Array of parameters passed to hook functions
 	 * @return bool True if no handler aborted the hook
 	 *
+	 * @since 1.22 A hook function is not required to return a value for
+	 *   processing to continue. Not returning a value (or explicitly
+	 *   returning null) is equivalent to returning true.
 	 * @throws MWException
 	 * @throws FatalError
 	 */
@@ -210,14 +213,7 @@ class Hooks {
 					'Detected bug in an extension! ' .
 					"Hook $func has invalid call signature; " . $badhookmsg
 				);
-			} elseif ( $retval === null ) {
-				// Null was returned. Error.
-				throw new MWException(
-					'Detected bug in an extension! ' .
-					"Hook $func failed to return a value; " .
-					'should return true to continue hook processing or false to abort.'
-				);
-			} elseif ( !$retval ) {
+			} elseif ( $retval === false ) {
 				wfProfileOut( 'hook: ' . $event );
 				// False was returned. Stop processing, but no error.
 				return false;