r98012 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r98011‎ | r98012 | r98013 >
Date:17:52, 24 September 2011
Author:ialex
Status:resolved (Comments)
Tags:
Comment:
Allow LogEventsList::showLogExtract() to get a Title object instead of having to do Title -> string -> Title conversion
Modified paths:
  • /trunk/phase3/includes/EditPage.php (modified) (history)
  • /trunk/phase3/includes/FileDeleteForm.php (modified) (history)
  • /trunk/phase3/includes/HistoryPage.php (modified) (history)
  • /trunk/phase3/includes/LogEventsList.php (modified) (history)
  • /trunk/phase3/includes/ProtectionForm.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialBlock.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialContributions.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialDeletedContributions.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMergeHistory.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMovepage.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialRevisiondelete.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUndelete.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUpload.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserrights.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ProtectionForm.php
——@@ -606,7 +606,7 @@
607607 function showLogExtract( &$out ) {
608608 # Show relevant lines from the protection log:
609609 $out->addHTML( Xml::element( 'h2', null, LogPage::logName( 'protect' ) ) );
610 - LogEventsList::showLogExtract( $out, 'protect', $this->mTitle->getPrefixedText() );
 610+ LogEventsList::showLogExtract( $out, 'protect', $this->mTitle );
611611 # Let extensions add other relevant log extracts
612612 wfRunHooks( 'ProtectionForm::showLogExtract', array($this->mArticle,$out) );
613613 }
Index: trunk/phase3/includes/EditPage.php
——@@ -807,7 +807,7 @@
808808 LogEventsList::showLogExtract(
809809 $wgOut,
810810 'block',
811 - $user->getUserPage()->getPrefixedText(),
 811+ $user->getUserPage(),
812812 '',
813813 array(
814814 'lim' => 1,
——@@ -830,7 +830,7 @@
831831 }
832832 # Give a notice if the user is editing a deleted/moved page...
833833 if ( !$this->mTitle->exists() ) {
834 - LogEventsList::showLogExtract( $wgOut, array( 'delete', 'move' ), $this->mTitle->getPrefixedText(),
 834+ LogEventsList::showLogExtract( $wgOut, array( 'delete', 'move' ), $this->mTitle,
835835 '', array( 'lim' => 10,
836836 'conds' => array( "log_action != 'revision'" ),
837837 'showIfEmpty' => false,
——@@ -1649,7 +1649,7 @@
16501650 # Then it must be protected based on static groups (regular)
16511651 $noticeMsg = 'protectedpagewarning';
16521652 }
1653 - LogEventsList::showLogExtract( $wgOut, 'protect', $this->mTitle->getPrefixedText(), '',
 1653+ LogEventsList::showLogExtract( $wgOut, 'protect', $this->mTitle, '',
16541654 array( 'lim' => 1, 'msgKey' => array( $noticeMsg ) ) );
16551655 }
16561656 if ( $this->mTitle->isCascadeProtected() ) {
——@@ -1667,7 +1667,7 @@
16681668 $wgOut->wrapWikiMsg( $notice, array( 'cascadeprotectedwarning', $cascadeSourcesCount ) );
16691669 }
16701670 if ( !$this->mTitle->exists() && $this->mTitle->getRestrictions( 'create' ) ) {
1671 - LogEventsList::showLogExtract( $wgOut, 'protect', $this->mTitle->getPrefixedText(), '',
 1671+ LogEventsList::showLogExtract( $wgOut, 'protect', $this->mTitle, '',
16721672 array( 'lim' => 1,
16731673 'showIfEmpty' => false,
16741674 'msgKey' => array( 'titleprotectedwarning' ),
Index: trunk/phase3/includes/HistoryPage.php
——@@ -117,7 +117,7 @@
118118 LogEventsList::showLogExtract(
119119 $wgOut,
120120 array( 'delete', 'move' ),
121 - $this->title->getPrefixedText(),
 121+ $this->title,
122122 '',
123123 array( 'lim' => 10,
124124 'conds' => array( "log_action != 'revision'" ),
Index: trunk/phase3/includes/LogEventsList.php
——@@ -789,7 +789,7 @@
790790 * @param $list LogEventsList
791791 * @param $types String or Array: log types to show
792792 * @param $performer String: the user who made the log entries
793 - * @param $title String: the page title the log entries are for
 793+ * @param $title String or Title: the page title the log entries are for
794794 * @param $pattern String: do a prefix search rather than an exact title match
795795 * @param $conds Array: extra conditions for the query
796796 * @param $year Integer: the year to start from
——@@ -909,15 +909,19 @@
910910 * Set the log reader to return only entries affecting the given page.
911911 * (For the block and rights logs, this is a user page.)
912912 *
913 - * @param $page String: Title name as text
 913+ * @param $page String or Title object: Title name
914914 * @param $pattern String
915915 */
916916 private function limitTitle( $page, $pattern ) {
917917 global $wgMiserMode;
918918
919 - $title = Title::newFromText( $page );
920 - if( strlen( $page ) == 0 || !$title instanceof Title ) {
921 - return false;
 919+ if ( $page instanceof Title ) {
 920+ $title = $page;
 921+ } else {
 922+ $title = Title::newFromText( $page );
 923+ if( strlen( $page ) == 0 || !$title instanceof Title ) {
 924+ return false;
 925+ }
922926 }
923927
924928 $this->title = $title->getPrefixedText();
Index: trunk/phase3/includes/FileDeleteForm.php
——@@ -236,7 +236,7 @@
237237 private function showLogEntries() {
238238 global $wgOut;
239239 $wgOut->addHTML( '<h2>' . htmlspecialchars( LogPage::logName( 'delete' ) ) . "</h2>\n" );
240 - LogEventsList::showLogExtract( $wgOut, 'delete', $this->title->getPrefixedText() );
 240+ LogEventsList::showLogExtract( $wgOut, 'delete', $this->title );
241241 }
242242
243243 /**
Index: trunk/phase3/includes/specials/SpecialUserrights.php
——@@ -606,6 +606,6 @@
607607 */
608608 protected function showLogFragment( $user, $output ) {
609609 $output->addHTML( Xml::element( 'h2', null, LogPage::logName( 'rights' ) . "\n" ) );
610 - LogEventsList::showLogExtract( $output, 'rights', $user->getUserPage()->getPrefixedText() );
 610+ LogEventsList::showLogExtract( $output, 'rights', $user->getUserPage() );
611611 }
612612 }
Index: trunk/phase3/includes/specials/SpecialUndelete.php
——@@ -1076,11 +1076,11 @@
10771077
10781078 # Show relevant lines from the deletion log:
10791079 $out->addHTML( Xml::element( 'h2', null, LogPage::logName( 'delete' ) ) . "\n" );
1080 - LogEventsList::showLogExtract( $out, 'delete', $this->mTargetObj->getPrefixedText() );
 1080+ LogEventsList::showLogExtract( $out, 'delete', $this->mTargetObj );
10811081 # Show relevant lines from the suppression log:
10821082 if( $this->getUser()->isAllowed( 'suppressionlog' ) ) {
10831083 $out->addHTML( Xml::element( 'h2', null, LogPage::logName( 'suppress' ) ) . "\n" );
1084 - LogEventsList::showLogExtract( $out, 'suppress', $this->mTargetObj->getPrefixedText() );
 1084+ LogEventsList::showLogExtract( $out, 'suppress', $this->mTargetObj );
10851085 }
10861086
10871087 if( $this->mAllowed && ( $haveRevisions || $haveFiles ) ) {
Index: trunk/phase3/includes/specials/SpecialRevisiondelete.php
——@@ -208,12 +208,12 @@
209209 # Show relevant lines from the deletion log
210210 $output->addHTML( "<h2>" . htmlspecialchars( LogPage::logName( 'delete' ) ) . "</h2>\n" );
211211 LogEventsList::showLogExtract( $output, 'delete',
212 - $this->targetObj->getPrefixedText(), '', array( 'lim' => 25, 'conds' => $qc ) );
 212+ $this->targetObj, '', array( 'lim' => 25, 'conds' => $qc ) );
213213 # Show relevant lines from the suppression log
214214 if( $user->isAllowed( 'suppressionlog' ) ) {
215215 $output->addHTML( "<h2>" . htmlspecialchars( LogPage::logName( 'suppress' ) ) . "</h2>\n" );
216216 LogEventsList::showLogExtract( $output, 'suppress',
217 - $this->targetObj->getPrefixedText(), '', array( 'lim' => 25, 'conds' => $qc ) );
 217+ $this->targetObj, '', array( 'lim' => 25, 'conds' => $qc ) );
218218 }
219219 }
220220
Index: trunk/phase3/includes/specials/SpecialContributions.php
——@@ -228,7 +228,7 @@
229229 LogEventsList::showLogExtract(
230230 $out,
231231 'block',
232 - $nt->getPrefixedText(),
 232+ $nt,
233233 '',
234234 array(
235235 'lim' => 1,
Index: trunk/phase3/includes/specials/SpecialUpload.php
——@@ -266,7 +266,7 @@
267267 $delNotice = ''; // empty by default
268268 if ( $desiredTitleObj instanceof Title && !$desiredTitleObj->exists() ) {
269269 LogEventsList::showLogExtract( $delNotice, array( 'delete', 'move' ),
270 - $desiredTitleObj->getPrefixedText(),
 270+ $desiredTitleObj,
271271 '', array( 'lim' => 10,
272272 'conds' => array( "log_action != 'revision'" ),
273273 'showIfEmpty' => false,
Index: trunk/phase3/includes/specials/SpecialDeletedContributions.php
——@@ -422,7 +422,7 @@
423423 LogEventsList::showLogExtract(
424424 $out,
425425 'block',
426 - $nt->getPrefixedText(),
 426+ $nt,
427427 '',
428428 array(
429429 'lim' => 1,
Index: trunk/phase3/includes/specials/SpecialBlock.php
——@@ -404,7 +404,7 @@
405405 LogEventsList::showLogExtract(
406406 $out,
407407 'block',
408 - $userpage->getPrefixedText(),
 408+ $userpage,
409409 '',
410410 array(
411411 'lim' => 10,
——@@ -419,7 +419,7 @@
420420 LogEventsList::showLogExtract(
421421 $out,
422422 'suppress',
423 - $userpage->getPrefixedText(),
 423+ $userpage,
424424 '',
425425 array(
426426 'lim' => 10,
Index: trunk/phase3/includes/specials/SpecialMovepage.php
——@@ -216,7 +216,7 @@
217217 }
218218 $out->addHTML( "<div class='mw-warning-with-logexcerpt'>\n" );
219219 $out->addWikiMsg( $noticeMsg );
220 - LogEventsList::showLogExtract( $out, 'protect', $this->oldTitle->getPrefixedText(), '', array( 'lim' => 1 ) );
 220+ LogEventsList::showLogExtract( $out, 'protect', $this->oldTitle, '', array( 'lim' => 1 ) );
221221 $out->addHTML( "</div>\n" );
222222 }
223223
——@@ -580,7 +580,7 @@
581581 function showLogFragment( $title ) {
582582 $out = $this->getOutput();
583583 $out->addHTML( Xml::element( 'h2', null, LogPage::logName( 'move' ) ) );
584 - LogEventsList::showLogExtract( $out, 'move', $title->getPrefixedText() );
 584+ LogEventsList::showLogExtract( $out, 'move', $title );
585585 }
586586
587587 function showSubpages( $title ) {
Index: trunk/phase3/includes/specials/SpecialMergeHistory.php
——@@ -236,7 +236,7 @@
237237
238238 # Show relevant lines from the deletion log:
239239 $out->addHTML( '<h2>' . htmlspecialchars( LogPage::logName( 'merge' ) ) . "</h2>\n" );
240 - LogEventsList::showLogExtract( $out, 'merge', $this->mTargetObj->getPrefixedText() );
 240+ LogEventsList::showLogExtract( $out, 'merge', $this->mTargetObj );
241241
242242 # When we submit, go by page ID to avoid some nasty but unlikely collisions.
243243 # Such would happen if a page was renamed after the form loaded, but before submit

Follow-up revisions

RevisionCommit summaryAuthorDate
r98431Per Aaron, fix for r98012: make Title object be handled correctly and documen...ialex19:03, 29 September 2011

Comments

#Comment by Aaron Schulz (talk | contribs)   20:22, 28 September 2011 
+	 * @param $page String or Title object: Title name

Can you use "String|null" instead?

In LogEventsList.php: 

696 	if ( $page != '')
697 	$urlParam['page'] = $page;

How does this work now?

showLogExtract() docs were not updated wrt to $page 

721 	if (!wfRunHooks('LogEventsListShowLogExtract', array(&$s, $types, $page, $user, $param))) {

Hooks docs need updating or this should make sure $page is a string.

#Comment by IAlex (talk | contribs)   19:04, 29 September 2011

Fixed in r98431.

Status & tagging log