r110179 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r110178‎ | r110179 | r110180 >
Date:22:44, 27 January 2012
Author:btongminh
Status:ok (Comments)
Tags:core, todo 
Comment:
Follow-up r83791: do not show size links for wikis which do not use a 404 transform, since this is a rather expensive operation, which can delay loading for several seconds.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/ImagePage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
——@@ -239,6 +239,7 @@
240240 the edit.
241241 * (bug 33902) Decoding %2B with mw.Uri.decode results in ' ' instead of +
242242 * (bug 33762) QueryPage-based special pages no longer misses *-summary message.
 243+* Other sizes links are no longer generated for wikis without a 404 thumbnail handler.
243244
244245 === API changes in 1.19 ===
245246 * Made action=edit less likely to return "unknownerror", by returning the actual error
Index: trunk/phase3/includes/ImagePage.php
——@@ -330,11 +330,14 @@
331331 }
332332 $msgsmall = wfMessage( 'show-big-image-preview' )->
333333 rawParams( $this->makeSizeLink( $params, $width, $height ) )->
334 - parse() . ' ' .
 334+ parse();
 335+ if ( count( $otherSizes ) && $this->displayImg->getRepo()->transformVia404 ) {
 336+ $msgsmall .= ' ' .
335337 Html::rawElement( 'span', array( 'class' => 'mw-filepage-other-resolutions' ),
336338 wfMessage( 'show-big-image-other' )->rawParams( $wgLang->pipeList( $otherSizes ) )->
337339 params( count( $otherSizes ) )->parse()
338340 );
 341+ }
339342 } elseif ( $width == 0 && $height == 0 ){
340343 # Some sort of audio file that doesn't have dimensions
341344 # Don't output a no hi res message for such a file

Sign-offs

UserFlagDate
CA SUNGHAN KIMinspected13:03, 10 November 2013 (struck 13:05, 10 November 2013)

Follow-up revisions

RevisionCommit summaryAuthorDate
r110187Use accessor method, ping r110179reedy23:19, 27 January 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r83791(bug 2581, bug 6834) Added links to thumbnail in several resolutions to the f...btongminh22:49, 12 March 2011

Comments

#Comment by Aaron Schulz (talk | contribs)   22:59, 27 January 2012

Use canTransformVia404().

#Comment by Raymond (talk | contribs)   09:09, 28 January 2012

This revision does not handle the case when using InstantCommons. I miss the other size links on translatewiki.net where InstantCommons is enabled.

#Comment by Aaron Schulz (talk | contribs)   10:14, 28 January 2012

Good catch. I think it needs an isLocal() check.

#Comment by Aaron Schulz (talk | contribs)   10:47, 28 January 2012

Actually that still wouldn't work for ForeignDB files. Since ForiegnAPIFile overrides transform() to do use handler->getTransform() it perhaps ForeignApiRepo can also override canTransformVia404() to be true...the whole classes there are a bit hacky.

#Comment by Bryan (talk | contribs)   19:53, 28 January 2012

What we really need is a way to determine whether a transform can be considered expensive. Overriding canTransformVia404() does not really sound like a good plan to me. I'm not sure what is the proper way to fix this though.

#Comment by Aaron Schulz (talk | contribs)   23:30, 28 January 2012

Well ForiegnAPIFile presupposes transform via 404, so it wouldn't be terrible. Another option would be to have a isTransormExpensive() type function, which is cleaner I guess.

#Comment by Aaron Schulz (talk | contribs)   22:22, 6 February 2012

Marking OK + todo. This isn't that big of an issue.

Status & tagging log