From: Max Semenik <maxsem@users.mediawiki.org>
Date: Thu, 9 Feb 2012 20:39:16 +0000 (+0000)
Subject: (bug 28936, bug 5280) Broken or invalid titles can't be removed from watchlist. Now... 
X-Git-Tag: 1.31.0-rc.0~24816
X-Git-Url: https://git.cyclocoop.org//%22?a=commitdiff_plain;h=586503f33c6562bf7b9fbbf66d5451319de76514;p=lhc%2Fweb%2Fwiklou.git

(bug 28936, bug 5280) Broken or invalid titles can't be removed from watchlist. Now titles are fixed or deleted, if unfixable, upon loading Special:EditWatchlist.
---

diff --git a/RELEASE-NOTES-1.19 b/RELEASE-NOTES-1.19
index d4eeaa9d37..0fa45728ef 100644
--- 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 0eead0e83e..26fb7d61b4 100644
--- 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 7efa2b7fb1..a7e97c8f81 100644
--- 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();