mike.lifeguard+bugs wrote:

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

Changed to Priority High. suppressredirect has been enabled on all WMF wikis for sysops ([[:bugzilla:14998]]), and therefor this bug becomes more urgent. This is a really big transparency issue, as no information is shown and there is no link to the move log (see (2) in the original bug description). Only if you go manually to Special:Log/move and search for the page name, you get the log (but only few people know how to do this, I guess).

W wrote:

Please see [http://en.wikipedia.org/w/index.php?title=User_talk:RHaworth&oldid=278845386#Sharai_parda this request] and [http://en.wikipedia.org/w/index.php?title=Special:Log&hide_patrol_log=0&page=sharai+parda this log record].

I think the problem arose because I did a move without redirect before the article had been marked patrolled.

W wrote:

I can confirm my comment above. Look at this log report: http://en.wikipedia.org/w/index.php?title=Special:NewPages&offset=200903230423&limit=1

Try marking Tony Issac as patrolled or removing him from the new pages list since there is no longer any article.

I'm not really sure how this relates to the feature request described above. Perhaps a separate bug report is adequate for that problem.

W wrote:

It relates because it is another result of the new (and excellent) move-without-leaving-redirect option.

blanchardb wrote:

It looks like any time a page is moved with redirect suppressed, its entry in Special:NewPages remains lingering as an unpatrolled page until it is bumped out as a month-old entry. The only way I see to fix this is to recreate the page, mark it as patrolled, then tag it for deletion as a test page.

So far, every time I've seen it happen, it was because the page has been moved to the creator's userspace. In one instance, the original location has been protected against recreation, and I had to ask an administrator to recreate the page just so that its entry in Special:NewPages could be suppressed.

I will make a separate bug report as suggested in Comment #5.

replaces showDeletionLog by a more general showLogs both in EditPage.php and Article.php

attachment showmovelog.patch ignored as obsolete

added keywords

mike.lifeguard+bugs wrote:

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

ayg wrote:

The patch seems to go to great lengths to only show moves and deletes. Wouldn't it be simpler (and possibly more future-proof) to just show all log entries for the page? The last one is always going to be the relevant move or deletion, anyway.

If there is some way to display multiple logs, it should surely be abstracted into LogPager, not done on the caller side and duplicated in two different places. Actually, it looks like you could already do something like

		$pager = new LogPager( $loglist, array('move', 'delete'), false, $this->mTitle->getPrefixedText() );

and it might magically work, from a glance at the source code. You'd want to adjust LogPager::limitType() to handle this more explicitly, specifically in the $wgLogRestrictions check on about line 547, but it looks like it would be a simple change.

Thanks for the pointers. I'm working on a new patch, but it's not ready yet, therefore I hereby remove the keywords...

uses an array to fetch the move log as well as the deletion log. Fixes limitType (hopefully)

I tried to implement the proposal of Simetrical. I must admit, I don't really understand parts of the code (lines 565 and 567/571 of the patched LogEventsList). Therefore this patch should not be applied without testing it thoroughly.

attachment showmovelog-newversion.patch ignored as obsolete

ayg wrote:

You should try to follow our coding style:

http://www.mediawiki.org/wiki/Manual:Coding_conventions

I haven't looked at the content of the patch, but hopefully I'll find time, since it's not too big.

ayg wrote:

• * @param $type String: A log type ('upload', 'delete', etc)

+ * @param $type Array: Log types ('upload', 'delete', etc)

"String or array" is more correct.

+ // If $type is not an array, make it an array
+ if ( !is_array( $type ) )
+ {
+ $type=array( "$type" );
+ }

This should just be:

		$type = (array)$type;

Also note some style issues here (opening brace is on the same line as the if, "=" should have spaces around it).

Also, $type should probably be renamed $types, it's a little confusing now.

• if( isset($wgLogRestrictions[$type]) && !$wgUser->isAllowed($wgLogRestrictions[$type]) ) {

+ foreach ( $type as $i => $t ) {
+ if( isset( $wgLogRestrictions[$t] ) && !$wgUser->isAllowed($wgLogRestrictions[$t]) ) {
+ unset( $type["$i"] );
+ }

You seem to have too much indentation here. $type["$i"] seems a bit odd to me, may as well just be $type[$i]. Also, maybe best to avoid the extra $i and do $type = array_diff( $type, array( $t ) ) instead of the unset. If $type were renamed $types, you could do foreach ( $types as $type ) instead of the slightly confusing $t.

+ if ( count($type)==0 ) {

Style:

		if ( count( $type ) == 0 ) {

We use lots of spaces . . .

• 'type' => 'delete',

+ 'type' => '',

• array( 'type' => 'delete', 'page' => $this->mTitle->getPrefixedText() )

+ array( 'type' => '', 'page' => $this->mTitle->getPrefixedText() )

Is there a reason you're doing 'type' => '' instead of just dropping type altogether? Does that not work?

-'recreate-deleted-warn' => "'''Warning: You are recreating a page that was previously deleted.'''
+'moveddeleted-notice' => 'This page has been moved and/or deleted.
+The move and deletion log for the page are provided below for reference.',
+'recreate-moveddeleted-warn' => "'''Warning: You are recreating a page that was previously moved and/or deleted.'''

Moving without redirect is very unlikely compared to deletion, so I think it would be best to keep these saying just "deleted" instead of "moved and/or deleted". If you're going to keep both, I'd say "deleted and/or moved without redirect", which a) puts the common case first and b) makes it clear when this actually shows up (you won't see it for pages that were moved normally and not deleted).

(Why did you change the order of these two messages?)

I don't see any other problems. With these fixed, I'd be willing to commit it, if no problems show up when I test it out.

Fixed (hopefully all :)) issues with the previous patch

attachment showmovelog-final.patch ignored as obsolete

ayg wrote:

Committed as r49821 with the following tweaks:

• * @param $type String: A log type ('upload', 'delete', etc)

+ * @param $type String or array: Log types ('upload', 'delete', etc)

$type -> $types (in r49822)

+ // If $type is not an array, make it an array

$type -> $types

+ $types = array_diff( $types, array( $t ) );

$t -> $type

+ if ( count( $types ) == 0 ) {
+ $types = '';
+ }

Removed, array() evaluates to boolean false, so this isn't needed.

Also note for the future that you shouldn't add RELEASE-NOTES changes to your patches -- they're easy for the committer to add, and they'll virtually always cause a conflict when the committer tries to apply the patch.

Thanks for committing the patch and your help & pointers, Simetrical.

herd wrote:

Looking at an example URL of a page moved with redirect suppression:

http://en.wikipedia.org/w/index.php?title=Flagship_Bank&redirect=no&action=edit&redlink=1
http://en.wikipedia.org/w/index.php?title=Flagship_Bank&redirect=no

The delete log is shown on both edit (create) and view, but the move log is only shown on view (which is very hard to get to, all redlinks including nstab-main point to edit).

Another example:
http://en.wikipedia.org/w/index.php?title=%22Sexy_Music%22_(Meat_Puppets_song)&action=edit&redlink=1
http://en.wikipedia.org/w/index.php?title=%22Sexy_Music%22_(Meat_Puppets_song)

r51041 broke this while fixing bug 18747. I'm working on a fix, but I'm stuck:

I want to negate the condition array('log_action'=>'revision') so that logs are shown only if they are not revisiondelete-logs, how do I do that? An ugly alternative would be to explicitly list every log_action (delete, restore, move, move_redir). (The disadvantage of the latter fix is, that in the future every new log_action of log_type delete/move has to be included manually)

(In reply to comment #20)

r51041 broke this while fixing bug 18747. I'm working on a fix, but I'm stuck:

I want to negate the condition array('log_action'=>'revision') so that logs are
shown only if they are not revisiondelete-logs, how do I do that? An ugly
alternative would be to explicitly list every log_action (delete, restore,
move, move_redir). (The disadvantage of the latter fix is, that in the future
every new log_action of log_type delete/move has to be included manually)

Just use "log_action != 'revision'"

Fixes bug after r51041 broke it.

Attached:

Just use "log_action != 'revision'"

Ooops. Easy fix. I focused too much on the array structure that was previously in place.

ayg wrote:

Committed as r51956 with whitespace fixes. See [[mw:Manual:Coding conventions]], we use spaces around all operators and inside all parentheses.