r103711 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r103710‎ | r103711 | r103712 >
Date:21:22, 19 November 2011
Author:hashar
Status:resolved (Comments)
Tags:core 
Comment:
(bug 1697) Special:Log make Title an User object when the log type expect it
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/specials/SpecialLog.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
——@@ -149,6 +149,7 @@
150150 * (bug 32086) Special:PermanentLink now show an error message when no subpage
151151 was specified.
152152 * (bug 30368) Special:Newpages now shows the new page name for moved pages
 153+* (bug 1697) The way to search blocked usernames in block log should be clearer
153154
154155 === API changes in 1.19 ===
155156 * (bug 19838) siprop=interwikimap can now use the interwiki cache.
Index: trunk/phase3/includes/specials/SpecialLog.php
——@@ -30,6 +30,17 @@
3131 */
3232 class SpecialLog extends SpecialPage {
3333
 34+ /**
 35+ * List log type for which the target is a user
 36+ * Thus if the given target is in NS_MAIN we can alter it to be an NS_USER
 37+ * Title user instead.
 38+ */
 39+ private $typeOnUser = array(
 40+ 'block',
 41+ 'newusers',
 42+ 'right',
 43+ );
 44+
3445 public function __construct() {
3546 parent::__construct( 'Log' );
3647 }
——@@ -85,6 +96,20 @@
8697 }
8798 }
8899
 100+ # Some log types are only for a 'User:' title but we might have been given
 101+ # only the username instead of the full title 'User:username'. This part try
 102+ # to lookup for a user by that name and eventually fix user input. See bug 1697.
 103+ if( in_array( $opts->getValue( 'type' ), $this->typeOnUser ) ) {
 104+ # ok we have a type of log which expect a user title.
 105+ $target = Title::newFromText( $opts->getValue( 'page' ) );
 106+ if( $target->getNamespace() === NS_MAIN ) {
 107+ # User forgot to add 'User:', we are adding it for him
 108+ $opts->setValue( 'page',
 109+ Title::makeTitleSafe( NS_USER, $opts->getValue( 'page' ) )
 110+ );
 111+ }
 112+ }
 113+
89114 $this->show( $opts, $qc );
90115 }
91116
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -2573,8 +2573,8 @@
25742574 'pubmedurl' => '//www.ncbi.nlm.nih.gov/pubmed/$1?dopt=Abstract', # do not translate or duplicate this message to other languages
25752575
25762576 # Special:Log
2577 -'specialloguserlabel' => 'User:',
2578 -'speciallogtitlelabel' => 'Title:',
 2577+'specialloguserlabel' => 'Performer:',
 2578+'speciallogtitlelabel' => 'Target (title or user):',
25792579 'log' => 'Logs',
25802580 'all-logs-page' => 'All public logs',
25812581 'alllogstext' => 'Combined display of all available logs of {{SITENAME}}.

Follow-up revisions

RevisionCommit summaryAuthorDate
r103791Special:Log title can be an invalid thus trigger a Fatal error...hashar22:39, 20 November 2011
r109025typo in r103711: right -> rightshashar10:36, 16 January 2012

Comments

#Comment by Johnduhart (talk | contribs)   21:25, 19 November 2011

Okay but what about extensions that expect users?

#Comment by Hashar (talk | contribs)   11:33, 20 November 2011

I am not sure I understand what is the issue there. That patch is merely to change 'Hashar' to 'User:Hashar' for log block that needs a user target.

#Comment by Catrope (talk | contribs)   11:36, 20 November 2011

Which part of this commit is even relevant to extensions? Are you talking about the UI change in Special:Log (which is only for block entries AIUI) or about the i18n changes?

#Comment by Johnduhart (talk | contribs)   16:00, 20 November 2011

$typeOnUser hardcodes what log types get the automatic conversion from NS_MAIN to NS_USER, meaning that extensions' log types thst expect a user for the target cannot benefit from this, and will cause confusion when this works for core and not extensions.

#Comment by Hashar (talk | contribs)   22:41, 20 November 2011

Maybe add a new accessor in SpecialLog to let extension append a type to $typeOnUser ? That variable might deserve a better name too.

#Comment by Nikerabbit (talk | contribs)   09:06, 21 November 2011

The new LogFormatter class seems to be most suitable place for this.

#Comment by Hashar (talk | contribs)   09:24, 3 January 2012

LogFormatter is for rendering. The patch above is to change our interpretation of a submitted title text. No way the LogFormatter could handle that :-)

#Comment by Legoktm (talk | contribs)   18:58, 21 November 2013

Hook added in gerrit:95075.

#Comment by Raymond (talk | contribs)   21:32, 20 November 2011

Seen on translatewiki:

PHP Fatal error: Call to a member function getNamespace() on a non-object in /www/w/includes/specials/SpecialLog.php on line 105
#Comment by Hashar (talk | contribs)   22:39, 20 November 2011

That one should be fixed with r103791.

#Comment by Brion VIBBER (talk | contribs)   02:04, 23 November 2011

Needs extensibility for the list of things that should take users; probably should also generalize to other NSs like files?

#Comment by Aaron Schulz (talk | contribs)   01:40, 13 January 2012

The type is 'rights' not 'right.

#Comment by Hashar (talk | contribs)   10:37, 16 January 2012

Typo fixed in r109025

Status & tagging log