Fix deprecated hooks not having a non-deprecated alternative
authorThiemo Mättig <thiemo.maettig@wikimedia.de>
Mon, 30 Jun 2014 14:48:08 +0000 (16:48 +0200)
committerThiemo Mättig <thiemo.maettig@wikimedia.de>
Mon, 30 Jun 2014 15:08:15 +0000 (17:08 +0200)
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
includes/specials/SpecialRecentchanges.php
includes/specials/SpecialRecentchangeslinked.php
includes/specials/SpecialWatchlist.php

index ad1ee36..0408182 100644 (file)
@@ -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
         *
index f770307..c0645e1 100644 (file)
@@ -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() );
        }
index e73cabc..6c617cd 100644 (file)
@@ -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 ) {
index 21a1f9b..1db1d04 100644 (file)
@@ -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
         *