(bug 28936, bug 5280) Broken or invalid titles can't be removed from watchlist. Now...

diff --git a/RELEASE-NOTES-1.19 b/RELEASE-NOTES-1.19
index d4eeaa9..0fa4572 100644 (file)
--- a/RELEASE-NOTES-1.19
+++ b/RELEASE-NOTES-1.19
@@ -246,6 +246,7 @@ production.
   getText() on a non-object
 * (bug 31676) Group dynamically inserted CSS into a single <style> tag, to work
   around a bug where not all styles were applied in Internet Explorer
+* (bug 28936, bug 5280) Broken or invalid titles can't be removed from watchlist.
 
 === API changes in 1.19 ===
 * Made action=edit less likely to return "unknownerror", by returning the actual error

diff --git a/includes/User.php b/includes/User.php
index 0eead0e..26fb7d6 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -2601,14 +2601,6 @@ class User {
                $this->invalidateCache();
        }
 
-       /**
-        * Cleans up watchlist by removing invalid entries from it
-        */
-       public function cleanupWatchlist() {
-               $dbw = wfGetDB( DB_MASTER );
-               $dbw->delete( 'watchlist', array( 'wl_namespace < 0', 'wl_user' => $this->getId() ), __METHOD__ );
-       }
-
        /**
         * Clear the user's notification timestamp for the given title.
         * If e-notif e-mails are on, they will receive notification mails on

diff --git a/includes/specials/SpecialEditWatchlist.php b/includes/specials/SpecialEditWatchlist.php
index 7efa2b7..a7e97c8 100644 (file)
--- a/includes/specials/SpecialEditWatchlist.php
+++ b/includes/specials/SpecialEditWatchlist.php
@@ -23,6 +23,8 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
 
        protected $toc;
 
+       private $badItems = array();
+
        public function __construct(){
                parent::__construct( 'EditWatchlist' );
        }
@@ -224,11 +226,15 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
                if( $res->numRows() > 0 ) {
                        foreach ( $res as $row ) {
                                $title = Title::makeTitleSafe( $row->wl_namespace, $row->wl_title );
-                               if( $title instanceof Title && !$title->isTalkPage() )
+                               if ( $this->checkTitle( $title, $row->wl_namespace, $row->wl_title )
+                                       && !$title->isTalkPage()
+                               ) {
                                        $list[] = $title->getPrefixedText();
+                               }
                        }
                        $res->free();
                }
+               $this->cleanupWatchlist();
                return $list;
        }
 
@@ -262,6 +268,60 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
                return $titles;
        }
 
+       /**
+        * Validates watchlist entry
+        *
+        * @param Title $title
+        * @param int $namespace
+        * @param String $dbKey 
+        * @return bool: Whether this item is valid
+        */
+       private function checkTitle( $title, $namespace, $dbKey ) {
+               if ( $title
+                       && ( $title->isExternal()
+                               || $title->getNamespace() < 0
+                       )
+               ) {
+                       $title = false; // unrecoverable
+               }
+               if ( !$title
+                       || $title->getNamespace() != $namespace
+                       || $title->getDBkey() != $dbKey
+               ) {
+                       $this->badItems[] = array( $title, $namespace, $dbKey );
+               }
+               return (bool)$title;
+       }
+
+       /**
+        * Attempts to clean up broken items
+        */
+       private function cleanupWatchlist() {
+               if ( count( $this->badItems ) ) {
+                       $dbw = wfGetDB( DB_MASTER );
+               }
+               foreach ( $this->badItems as $row ) {
+                       list( $title, $namespace, $dbKey ) = $row;
+                       wfDebug( "User {$this->getUser()} has broken watchlist item ns($namespace):$dbKey, "
+                               . ( $title ? 'cleaning up' : 'deleting' ) . ".\n"
+                       );
+
+                       $dbw->delete( 'watchlist',
+                               array(
+                                       'wl_user' => $this->getUser()->getId(),
+                                       'wl_namespace' => $namespace,
+                                       'wl_title' => $dbKey,
+                               ),
+                               __METHOD__
+                       );
+
+                       // Can't just do an UPDATE instead of DELETE/INSERT due to unique index
+                       if ( $title ) {
+                               $this->getUser()->addWatch( $title );
+                       }
+               }
+       }
+
        /**
         * Remove all titles from a user's watchlist
         */
@@ -375,30 +435,25 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
                $fields = array();
                $count = 0;
 
-               $haveInvalidNamespaces = false;
                foreach( $this->getWatchlistInfo() as $namespace => $pages ){
-                       if ( $namespace < 0 ) {
-                               $haveInvalidNamespaces = true;
-                               continue;
+                       if ( $namespace >= 0 ) {
+                               $fields['TitlesNs'.$namespace] = array(
+                                       'class' => 'EditWatchlistCheckboxSeriesField',
+                                       'options' => array(),
+                                       'section' => "ns$namespace",
+                               );
                        }
 
-                       $fields['TitlesNs'.$namespace] = array(
-                               'class' => 'EditWatchlistCheckboxSeriesField',
-                               'options' => array(),
-                               'section' => "ns$namespace",
-                       );
-
                        foreach( array_keys( $pages ) as $dbkey ){
                                $title = Title::makeTitleSafe( $namespace, $dbkey );
-                               $text = $this->buildRemoveLine( $title );
-                               $fields['TitlesNs'.$namespace]['options'][$text] = $title->getEscapedText();
-                               $count++;
+                               if ( $this->checkTitle( $title, $namespace, $dbkey ) ) {
+                                       $text = $this->buildRemoveLine( $title );
+                                       $fields['TitlesNs'.$namespace]['options'][$text] = $title->getEscapedText();
+                                       $count++;
+                               }
                        }
                }
-               if ( $haveInvalidNamespaces ) {
-                       wfDebug( "User {$this->getContext()->getUser()->getId()} has invalid watchlist entries, cleaning up...\n" );
-                       $this->getContext()->getUser()->cleanupWatchlist();
-               }
+               $this->cleanupWatchlist();
 
                if ( count( $fields ) > 1 && $count > 30 ) {
                        $this->toc = Linker::tocIndent();