-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

robchurch@svn.wikimedia.org wrote:

Move all watchlist editing into WatchlistEditor class, integrating "raw editing" from ExportWatchlist extension, per Brion. Spiffed up the existing editor and "clear" interface; all three editors better integrated from a UI perspective.

Looks good!

• 			$titles = $this->extractTitles( $request->getText( 'titles' ) );
  

• 			$this->clearWatchlist( $user );
  

• 			$this->watchTitles( $titles, $user );
  

• 			$user->invalidateCache();
  

This raw edit save will reset the last-seen timestamps for all watched pages, which could hurt a bit for people using email notification.

Doing an array diff and only adding/removing the changed items should work cleaner for that case, plus save some DB activity when making small changes to a large watchlist.

[snip]

• 	$rows[] = array(
  

• 		'wl_user' => $user->getId(),
  

• 		'wl_namespace' => ( $title->getNamespace() & ~1 ),
  

• 		'wl_title' => $title->getDBkey(),
  

• 		'wl_notificationtimestamp' => null,
  

• 	);
  

• 	$rows[] = array(
  

• 		'wl_user' => $user->getId(),
  

• 		'wl_namespace' => ( $title->getNamespace() | 1 ),
  

• 		'wl_title' => $title->getDBkey(),
  

• 		'wl_notificationtimestamp' => null,
  

• 	);
  

This low-level code makes some surprises in corner cases. :)

If you pass a Special: or Media: title, it accepts them and treats them as an article-talk pair. Interwiki titles are also accepted, treated as normal pages.

Probably these unwatchable titles should get stripped out of the stream, either here or at the high-level UI end.

I'd also recommend moving the low-level DB routines over to WatchedItem, which IIRC already has some batch-change functions.

• }
  

• }
• /**

• * Show the standard watchlist editing form
  

• *
  

• * @param OutputPage $output
  

• * @param User $user
  

• */
  

• private function showNormalForm( $output, $user ) {

• if( ( $count = $this->showItemCount( $output, $user ) ) > 0 ) {
  

• 	$self = SpecialPage::getTitleFor( 'Watchlist' );
  

• 	$form  = Xml::openElement( 'form', array( 'method' => 'post',
  

• 		'action' => $self->getLocalUrl( 'action=edit' ) ) );
  

• 	$form .= Xml::hidden( 'token', $user->editToken( 'watchlistedit' ) );
  

• 	$form .= '<fieldset><legend>' . wfMsgHtml( 'watchlistedit-normal-legend' ) . '</legend>';
  

• 	$form .= wfMsgExt( 'watchlistedit-normal-explain', 'parse' );
  

• 	foreach( $this->getWatchlist( $user ) as $namespace => $pages ) {
  

• 		$form .= '<h2>' . $this->getNamespaceHeading( $namespace ) . '</h2>';
  

• 		$form .= '<ul>';
  

• 		foreach( $pages as $dbkey => $redirect ) {
  

• 			$title = Title::makeTitleSafe( $namespace, $dbkey );
  

• 			$form .= $this->buildRemoveLine( $title, $redirect, $user->getSkin() );
  

The makeTitleSafe() reminds me of the possibility of invalid entries in the watchlist table; those may not roundtrip nicely through the raw edit form. Just something to consider... automatic cleanup (for instance when namespaces change in weird ways) might be appropriate.

+'watchlistedit-normal-explain' => 'Titles on your watchlist are shown below. To remove a title, check

• the box next to it, and click Remove Titles. You can also [[Special:Watchlist/raw|edit the raw list]],
• or [[Special:Watchlist/clear|remove all titles]].',

The mode links here feel buried in the UI text to me; it's not obvious to me that it's there. It might be interesting to try a more tab-like link here, maybe just simple links like on Special:Allmessages

- -- brion vibber (brion @ wikimedia.org)