r80432 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r80431‎ | r80432 | r80433 >
Date:01:16, 17 January 2011
Author:bawolff
Status:resolved (Comments)
Tags:
Comment:
(bug 26737; follow-up r70415) Make new category stuff play nice with __NOGALLERY__

This changes it so non-gallery cat pages with images, put the images in a list (like the
other sections), but in its own section, instead of as part of the pages section like it
used to be.
Modified paths:
  • /trunk/phase3/includes/CategoryPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/CategoryPage.php
——@@ -89,6 +89,7 @@
9090 $articles, $articles_start_char,
9191 $children, $children_start_char,
9292 $showGallery, $gallery,
 93+ $imgsNoGalley, $imgsNoGallery_start_char,
9394 $skin;
9495 # Category object for this page
9596 private $cat;
——@@ -155,6 +156,9 @@
156157 if ( $this->showGallery ) {
157158 $this->gallery = new ImageGallery();
158159 $this->gallery->setHideBadImages();
 160+ } else {
 161+ $this->imgsNoGallery = array();
 162+ $this->imgsNoGallery_start_char = array();
159163 }
160164 }
161165
——@@ -217,6 +221,7 @@
218222 * Add a page in the image namespace
219223 */
220224 function addImage( Title $title, $sortkey, $pageLength, $isRedirect = false ) {
 225+ global $wgContLang;
221226 if ( $this->showGallery ) {
222227 $flip = $this->flip['file'];
223228 if ( $flip ) {
——@@ -225,7 +230,18 @@
226231 $this->gallery->add( $title );
227232 }
228233 } else {
229 - $this->addPage( $title, $sortkey, $pageLength, $isRedirect );
 234+ $this->imgsNoGallery[] = $isRedirect
 235+ ? '<span class="redirect-in-category">' .
 236+ $this->getSkin()->link(
 237+ $title,
 238+ null,
 239+ array(),
 240+ array(),
 241+ array( 'known', 'noclasses' )
 242+ ) . '</span>'
 243+ : $this->getSkin()->link( $title );
 244+
 245+ $this->imgsNoGallery_start_char[] = $wgContLang->convert( $wgContLang->firstLetterForLists( $sortkey ) );
230246 }
231247 }
232248
——@@ -257,6 +273,10 @@
258274 $this->articles = array_reverse( $this->articles );
259275 $this->articles_start_char = array_reverse( $this->articles_start_char );
260276 }
 277+ if ( !$this->showGallery && $this->flip['file'] ) {
 278+ $this->imgsNoGallery = array_reverse( $this->imgsNoGallery );
 279+ $this->imgsNoGallery_start_char = array_reverse( $this->imgsNoGallery_start_char );
 280+ }
261281 }
262282
263283 function doCategoryQuery() {
——@@ -318,7 +338,7 @@
319339 if ( $title->getNamespace() == NS_CATEGORY ) {
320340 $cat = Category::newFromRow( $row, $title );
321341 $this->addSubcategoryObject( $cat, $rawSortkey, $row->page_len );
322 - } elseif ( $this->showGallery && $title->getNamespace() == NS_FILE ) {
 342+ } elseif ( $title->getNamespace() == NS_FILE ) {
323343 $this->addImage( $title, $rawSortkey, $row->page_len, $row->page_is_redirect );
324344 } else {
325345 $this->addPage( $title, $rawSortkey, $row->page_len, $row->page_is_redirect );
——@@ -382,16 +402,20 @@
383403
384404 function getImageSection() {
385405 $r = '';
386 - if ( $this->showGallery && ! $this->gallery->isEmpty() ) {
 406+ $rescnt = $this->showGallery ? $this->gallery->count() : count( $this->imgsNoGallery );
 407+ if ( $rescnt > 0 ) {
387408 $dbcnt = $this->cat->getFileCount();
388 - $rescnt = $this->gallery->count();
389409 $countmsg = $this->getCountMessage( $rescnt, $dbcnt, 'file' );
390410
391411 $r .= "<div id=\"mw-category-media\">\n";
392412 $r .= '<h2>' . wfMsg( 'category-media-header', htmlspecialchars( $this->title->getText() ) ) . "</h2>\n";
393413 $r .= $countmsg;
394414 $r .= $this->getSectionPagingLinks( 'file' );
395 - $r .= $this->gallery->toHTML();
 415+ if ( $this->showGallery ) {
 416+ $r .= $this->gallery->toHTML();
 417+ } else {
 418+ $r .= $this->formatList( $this->imgsNoGallery, $this->imgsNoGallery_start_char );
 419+ }
396420 $r .= $this->getSectionPagingLinks( 'file' );
397421 $r .= "\n</div>";
398422 }
——@@ -594,7 +618,7 @@
595619 # know the right figure.
596620 # 3) We have no idea.
597621 $totalrescnt = count( $this->articles ) + count( $this->children ) +
598 - ( $this->showGallery ? $this->gallery->count() : 0 );
 622+ ( $this->showGallery ? $this->gallery->count() : count( $this->imgsNoGallery ) );
599623
600624 # Check if there's a "from" or "until" for anything
601625 $fromOrUntil = false;

Follow-up revisions

RevisionCommit summaryAuthorDate
r80433(follow-up r70415) Fixes the function that determines if category counts are ...bawolff02:27, 17 January 2011
r80590Follow up r80432. Don't use 'known', 'noclasses' when making the links on cat...bawolff21:51, 19 January 2011
r806781.17: MFT category collation changes: r80432, r80443, r80590, r80614, r80615,...catrope03:16, 21 January 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r70415Enable new category sort by default...simetrical20:50, 3 August 2010

Comments

#Comment by Nikerabbit (talk | contribs)   07:22, 17 January 2011 
+			$this->imgsNoGallery[] = $isRedirect
+			? '<span class="redirect-in-category">' .
+				$this->getSkin()->link(
+					$title,
+					null,
+					array(),
+					array(),
+					array( 'known', 'noclasses' )
+				) . '</span>'
+			: $this->getSkin()->link( $title );

If the stuff is properly preloaded, you don't need this kind of trickery. Instead you can use $this->getSkin()->link( $title ); and have the linker insert classes like mw-redirect to the link.

#Comment by Catrope (talk | contribs)   01:42, 19 January 2011

"preloading properly" can be done by throwing all the titles in a LinkBatch, with something like. $lb = new LinkBatch; foreach ( $titles as $title ) { $lb->addObj( $title ); } $lb->execute(); which will cause the existence statuses and some other metadata (including redirect status) of all added titles to be retrieved in one query and cached for subsequent exists() calls and the like.

This rev is OK otherwise, can be marked resolved once Nike's comment has been addressed.

#Comment by Bawolff (talk | contribs)   04:41, 19 January 2011

Since we're already querying the page table in CategoryPage, would it be more efficient to add page_latest to the list fields we retrieve, and then use the addResultToCache method of LinkBatch?

#Comment by Nikerabbit (talk | contribs)   06:56, 19 January 2011

Yep. Title::newFromRow might be the easiest way here, see what I did to Special:AllPages in r80394.

#Comment by Bawolff (talk | contribs)   22:24, 19 January 2011

Hmm, appears they were already being loaded directly from the retrieved row. Fixed in r80590.

I'm setting this revision back to new. I'm not sure if I am allowed to set my own revisions to "resolved", even if it was stated previously it was ok otherwise(?)

#Comment by Catrope (talk | contribs)   01:01, 21 January 2011

No, don't set your own revisions to resolved or ok. When all fixme's on a revision have been addressed, set it back to new.

Status & tagging log