From 08fee4ce2f1bbf362c8422a3e306a905c8775094 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thiemo=20M=C3=A4ttig?= Date: Mon, 30 Jun 2014 16:48:08 +0200 Subject: [PATCH] Fix deprecated hooks not having a non-deprecated alternative The implementation in ChangesListSpecialPage::doMainQuery() is dead code. It's never executed because the relevant subclasses override the method completely without ever calling parent::doMainQuery(). This means that the deprecation of the hooks in the subclasses leaves us with no alternative. Which is a real problem in Wikibase, see I27716275e966147ee26d81d8ce3f14951937e718. The same deprecation in ChangesListSpecialPage::getCustomFilters() works because all suclasses I'm aware of call the parent method. But please note that extensions may still break if they do this different. I can not checked them all. If you want to be sure you should just revert patch I9cceda5d2dcfc53c852c5682c466b48ad8f31202 that introduced this non-functional hook. I'm sorry to say that but I wonder how this passed review. Change-Id: I7ba3ea64cb145c06011a856e5b56399da4f42339 --- includes/specialpage/ChangesListSpecialPage.php | 11 +++++++++-- includes/specials/SpecialRecentchanges.php | 14 +++++++++++--- includes/specials/SpecialRecentchangeslinked.php | 14 +++++++++++--- includes/specials/SpecialWatchlist.php | 13 ++++++++++--- 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/includes/specialpage/ChangesListSpecialPage.php b/includes/specialpage/ChangesListSpecialPage.php index ad1ee36a8d..04081825a9 100644 --- a/includes/specialpage/ChangesListSpecialPage.php +++ b/includes/specialpage/ChangesListSpecialPage.php @@ -290,8 +290,8 @@ abstract class ChangesListSpecialPage extends SpecialPage { '' ); - if ( !wfRunHooks( 'ChangesListSpecialPageQuery', - array( $this->getName(), &$tables, &$fields, &$conds, &$query_options, &$join_conds, $opts ) ) + if ( !$this->runMainQueryHook( $tables, $fields, $conds, $query_options, $join_conds, + $opts ) ) { return false; } @@ -308,6 +308,13 @@ abstract class ChangesListSpecialPage extends SpecialPage { ); } + protected function runMainQueryHook( &$tables, &$fields, &$conds, &$query_options, &$join_conds, $opts ) { + return wfRunHooks( + 'ChangesListSpecialPageQuery', + array( $this->getName(), &$tables, &$fields, &$conds, &$query_options, &$join_conds, $opts ) + ); + } + /** * Return a DatabaseBase object for reading * diff --git a/includes/specials/SpecialRecentchanges.php b/includes/specials/SpecialRecentchanges.php index f770307801..c0645e130b 100644 --- a/includes/specials/SpecialRecentchanges.php +++ b/includes/specials/SpecialRecentchanges.php @@ -229,9 +229,8 @@ class SpecialRecentChanges extends ChangesListSpecialPage { $opts['tagfilter'] ); - if ( !wfRunHooks( 'SpecialRecentChangesQuery', - array( &$conds, &$tables, &$join_conds, $opts, &$query_options, &$fields ), - '1.23' ) + if ( !$this->runMainQueryHook( $tables, $fields, $conds, $query_options, $join_conds, + $opts ) ) { return false; } @@ -255,6 +254,15 @@ class SpecialRecentChanges extends ChangesListSpecialPage { return $rows; } + protected function runMainQueryHook( &$tables, &$fields, &$conds, &$query_options, &$join_conds, $opts ) { + return parent::runMainQueryHook( $tables, $fields, $conds, $query_options, $join_conds, $opts ) + && wfRunHooks( + 'SpecialRecentChangesQuery', + array( &$conds, &$tables, &$join_conds, $opts, &$query_options, &$fields ), + '1.23' + ); + } + public function outputFeedLinks() { $this->addFeedLinks( $this->getFeedQuery() ); } diff --git a/includes/specials/SpecialRecentchangeslinked.php b/includes/specials/SpecialRecentchangeslinked.php index e73cabce79..6c617cdd9f 100644 --- a/includes/specials/SpecialRecentchangeslinked.php +++ b/includes/specials/SpecialRecentchangeslinked.php @@ -109,9 +109,8 @@ class SpecialRecentChangesLinked extends SpecialRecentChanges { $opts['tagfilter'] ); - if ( !wfRunHooks( 'SpecialRecentChangesQuery', - array( &$conds, &$tables, &$join_conds, $opts, &$query_options, &$select ), - '1.23' ) + if ( !$this->runMainQueryHook( $conds, $tables, $join_conds, $opts, $query_options, + $select ) ) { return false; } @@ -221,6 +220,15 @@ class SpecialRecentChangesLinked extends SpecialRecentChanges { return $res; } + protected function runMainQueryHook( &$tables, &$select, &$conds, &$query_options, &$join_conds, $opts ) { + return parent::runMainQueryHook( $tables, $select, $conds, $query_options, $join_conds, $opts ) + && wfRunHooks( + 'SpecialRecentChangesQuery', + array( &$conds, &$tables, &$join_conds, $opts, &$query_options, &$select ), + '1.23' + ); + } + function setTopText( FormOptions $opts ) { $target = $this->getTargetTitle(); if ( $target ) { diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php index 21a1f9b816..1db1d047e7 100644 --- a/includes/specials/SpecialWatchlist.php +++ b/includes/specials/SpecialWatchlist.php @@ -279,9 +279,7 @@ class SpecialWatchlist extends ChangesListSpecialPage { '' ); - wfRunHooks( 'SpecialWatchlistQuery', - array( &$conds, &$tables, &$join_conds, &$fields, $opts ), - '1.23' ); + $this->runMainQueryHook( $tables, $fields, $conds, $query_options, $join_conds, $opts ); return $dbr->select( $tables, @@ -293,6 +291,15 @@ class SpecialWatchlist extends ChangesListSpecialPage { ); } + protected function runMainQueryHook( &$tables, &$fields, &$conds, &$query_options, &$join_conds, $opts ) { + return parent::runMainQueryHook( $tables, $fields, $conds, $query_options, $join_conds, $opts ) + && wfRunHooks( + 'SpecialWatchlistQuery', + array( &$conds, &$tables, &$join_conds, &$fields, $opts ), + '1.23' + ); + } + /** * Return a DatabaseBase object for reading * -- 2.20.1