robchur wrote:

Watching deletions added in r18381.

robchur wrote:

I'm inclined to make rollback reuse the existing "watchdefault" preference,
since it's a special case of editing, but I suspect there'll be an uproar if
people's watchlists get flooded. Opinions?

I'd rather it be seperate, as some people do a good deal of RC patrol reverting
random vandalism but also make normal, slower, significant edits to pages they
care about, and it could indeed cause flooding.

ayg wrote:

And watchdefault can be manually overridden on the edit page with a checkbox,
which it presumably couldn't for rollback.

Changed component to "Watchlist"

*** Bug 17481 has been marked as a duplicate of this bug. ***

Proposed patch: Add user preference 'watchrollback'

I assume, nobody is currently working on this, but as it seems quite straightforward to implement, I gave it a try.

Per comment #3 and #4 this patch adds a separate user preference for watching rolled back pages automatically. To make the display page after the rollback work properly, I had to add the methods watch() and unwatch() to the Title class, so $wgTitle->userIsWatching() gives the correct result afterwards. Actually there are some more places, that call $wgUser->addWatch( $title ) which could be replaced now by $title->watch(), but I didn't want to add too much clutter to the patch, so I left it for now.

Of course, any comments are appreciated.

attachment bug4488.patch ignored as obsolete

Proposed patch seems to include a strange movement of watch/unwatch logic to the Title object. This doesn't make sense, because you watch the article, not the title.

• # If there were errors, bail out now
• if( !empty( $errors ) )
• return $errors;

• return $this->commitRollback($fromP, $summary, $bot, $resultDetails);

+ if( empty( $errors ) ) {
+ $errors = $this->commitRollback($fromP, $summary, $bot, $resultDetails);
+ }
+ if( empty( $errors ) && $wgUser->getOption( 'watchrollback' ) && !$this->mTitle->userIsWatching() ) {
+ $this->doWatch();
+ }
+ return $errors;

I think I like the early bail-out better. It avoids the clutter of checking empty($errors).

Also, you should check count($errors)==0, not empty($errors). empty() is used for a cast to bool with warnings suppressed. If you didn't want to do that, you should use count() explicitly.

It would be cool if you could work the 'create' toggle into the rest of the logic in Preferences, which is quite elegant compared to the current version.

Otherwise, the patch is pretty good. I may fix this stuff up over the coming days and commit it, if you don't get to it first.

Updated patch for new pref system

Oops, didn't realize someone commented on this one :D

(In reply to comment #8)

Proposed patch seems to include a strange movement of watch/unwatch logic to
the Title object. This doesn't make sense, because you watch the article, not
the title.

It's rather a small addition than a move. The problem is that the Title object caches, whether the page is watched or not in its member ->mWatched. Currenty, when we watch/unwatch a page, this member is not updated, so any following calls of $title->userIsWatching() will return the wrong result.
So I changed the calls from $wgUser->addWatch( $title ) to $title->watch(), which will update the member var and call $wgUser->addWatch( $this ) itself.

• # If there were errors, bail out now
• if( !empty( $errors ) )
• return $errors;

• return $this->commitRollback($fromP, $summary, $bot,

$resultDetails);
+ if( empty( $errors ) ) {
+ $errors = $this->commitRollback($fromP, $summary, $bot,
$resultDetails);
+ }
+ if( empty( $errors ) && $wgUser->getOption( 'watchrollback' )
&& !$this->mTitle->userIsWatching() ) {
+ $this->doWatch();
+ }
+ return $errors;

I think I like the early bail-out better. It avoids the clutter of checking
empty($errors).

I moved the second if-Statement into the first one now, so I think there shouldn't be a difference anymore.

Also, you should check count($errors)==0, not empty($errors). empty() is used
for a cast to bool with warnings suppressed. If you didn't want to do that, you
should use count() explicitly.

The existing code uses empty, so I didn't really want to change that.

It would be cool if you could work the 'create' toggle into the rest of the
logic in Preferences, which is quite elegant compared to the current version.

With the new pref system this was even easier :)

Attached:

john wrote:

Thank for submitting a patch to MediaWiki. I've reviewed the patch, and I don't think moving watch functions into the Title class is the best move here. It would be preferred to update the value of $mWatched from the User class.

FYI: The Extension Echo does this with rollbacks and undos.

(In reply to db from comment #11)

FYI: The Extension Echo does this with rollbacks and undos.

I don't think it does.

When visiting the preferences page [[Special:Preferences#mw-prefsection-echo]] there is a option "Edit revert" with the tooltip "Notify me when someone reverts an edit I made, by using the undo or rollback tool.", which I mention.

It is not directly a watch, but you will get a echo ping and can see the rollback/undo. Not perfectly what was requested, but thats why I only added it as info, not changed the state.

He's asking for pages that he rolls back to be added to his watchlist, not to be told when someone rolls him back.

Change 146440 had a related patch set uploaded by Ebe123:
Create preferences to watchlist pages after rollbacking and undoing

https://gerrit.wikimedia.org/r/146440

Change 146440 merged by Legoktm:
Create preference to watchlist pages after rollbacking

https://gerrit.wikimedia.org/r/146440

I hope it's fixed now. (And it should be)

Thanks for fixing this, it would have fit well in https://www.mediawiki.org/wiki/MediaWiki_1.23#Notifications :-)
This also makes it easier for sysadmins to enable "watchdefault" by default if they wish.