r84659 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r84658‎ | r84659 | r84660 >
Date:01:44, 24 March 2011
Author:aaron
Status:resolved (Comments)
Tags:
Comment:
* Replaced crufty BeforeParserMakeImageLinkObj/BeforeGalleryFindFile hooks with BeforeParserFetchFileAndTile hook
* Updated the only calling extension (these was basically single-purpose hooks)
Modified paths:
  • /trunk/extensions/FlaggedRevs/FlaggedRevs.hooks.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/FlaggedRevs.php (modified) (history)
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/ImageGallery.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)

Diff [purge]

Index: trunk/phase3/docs/hooks.txt
@@ -550,13 +550,6 @@
551551 Change $bad and return false to override. If an image is "bad", it is not
552552 rendered inline in wiki pages or galleries in category pages.
553553
554 -'BeforeGalleryFindFile': before an image is fetched for a gallery
555 -&$gallery,: the gallery object
556 -&$nt: the image title
557 -&$time: image timestamp (used to specify the file)
558 -&$descQuery: query string to add to thumbnail URL
559 -&$sha1: image base 36 sha1 (used to specify the file, $nt will be ignored if this is set)
560 -
561554 'BeforeInitialize': before anything is initialized in performRequestForTitle()
562555 &$title: Title being used for request
563556 &$article: The associated Article object
@@ -569,20 +562,19 @@
570563 &$out: OutputPage object
571564 &$skin: Skin object
572565
 566+'BeforeParserFetchFileAndTitle': before an image is rendered by Parser
 567+&$parser: Parser object
 568+&$nt: the image title
 569+&$time: the image timestamp (use '0' to force a broken thumbnail)
 570+&$sha1: image base 36 sha1 (used to specify the file, $nt will be ignored if this is set)
 571+&$descQuery: query string to add to thumbnail URL
 572+
573573 'BeforeParserFetchTemplateAndtitle': before a template is fetched by Parser
574574 &$parser: Parser object
575575 &$title: title of the template
576576 &$skip: skip this template and link it?
577577 &$id: the id of the revision being parsed
578578
579 -'BeforeParserMakeImageLinkObj': before an image is rendered by Parser
580 -&$parser: Parser object
581 -&$nt: the image title
582 -&$skip: skip this image and link it?
583 -&$time: the image timestamp
584 -&$descQuery: query string to add to thumbnail URL
585 -&$sha1: image base 36 sha1 (used to specify the file, $nt will be ignored if this is set)
586 -
587579 'BeforeParserrenderImageGallery': before an image gallery is rendered by Parser
588580 &$parser: Parser object
589581 &$ig: ImageGallery object
Index: trunk/phase3/includes/parser/Parser.php
@@ -1909,19 +1909,14 @@
19101910 if ( $ns == NS_MEDIA ) {
19111911 wfProfileIn( __METHOD__."-media" );
19121912 # Give extensions a chance to select the file revision for us
1913 - $skip = $time = $sha1 = $descQuery = false;
1914 - wfRunHooks( 'BeforeParserMakeImageLinkObj',
1915 - array( &$this, &$nt, &$skip, &$time, &$descQuery, &$sha1 ) );
1916 - if ( $skip ) {
1917 - $this->mOutput->addImage( $nt->getDBkey(), null, null ); // register
1918 - $link = $sk->link( $nt );
1919 - } else {
1920 - # Fetch and register the file (file title may be different via hooks)
1921 - list( $file, $nt ) = $this->fetchFileAndTitle( $nt, $time, $sha1 );
1922 - $link = $sk->makeMediaLinkFile( $nt, $file, $text );
1923 - }
 1913+ $time = $sha1 = $descQuery = false;
 1914+ wfRunHooks( 'BeforeParserFetchFileAndTitle',
 1915+ array( &$this, &$nt, &$time, &$sha1, &$descQuery ) );
 1916+ # Fetch and register the file (file title may be different via hooks)
 1917+ list( $file, $nt ) = $this->fetchFileAndTitle( $nt, $time, $sha1 );
19241918 # Cloak with NOPARSE to avoid replacement in replaceExternalLinks
1925 - $s .= $prefix . $this->armorLinks( $link ) . $trail;
 1919+ $s .= $prefix . $this->armorLinks(
 1920+ $sk->makeMediaLinkFile( $nt, $file, $text ) ) . $trail;
19261921 wfProfileOut( __METHOD__."-media" );
19271922 continue;
19281923 }
@@ -4702,15 +4697,12 @@
47034698 $sk = $this->mOptions->getSkin( $this->mTitle );
47044699
47054700 # Give extensions a chance to select the file revision for us
4706 - $skip = $time = $sha1 = $descQuery = false;
4707 - wfRunHooks( 'BeforeParserMakeImageLinkObj',
4708 - array( &$this, &$title, &$skip, &$time, &$descQuery, &$sha1 ) );
4709 - if ( $skip ) {
4710 - $this->mOutput->addImage( $title->getDBkey(), null, null ); // register
4711 - return $sk->link( $title );
4712 - }
 4701+ $time = $sha1 = $descQuery = false;
 4702+ wfRunHooks( 'BeforeParserFetchFileAndTitle',
 4703+ array( &$this, &$title, &$time, &$sha1, &$descQuery ) );
47134704 # Fetch and register the file (file title may be different via hooks)
47144705 list( $file, $title ) = $this->fetchFileAndTitle( $title, $time, $sha1 );
 4706+
47154707 # Get parameter map
47164708 $handler = $file ? $file->getHandler() : false;
47174709
Index: trunk/phase3/includes/ImageGallery.php
@@ -239,9 +239,8 @@
240240 }
241241
242242 $attribs = Sanitizer::mergeAttributes(
243 - array(
244 - 'class' => 'gallery'),
245 - $this->mAttribs );
 243+ array( 'class' => 'gallery' ), $this->mAttribs );
 244+
246245 $s = Xml::openElement( 'ul', $attribs );
247246 if ( $this->mCaption ) {
248247 $s .= "\n\t<li class='gallerycaption'>{$this->mCaption}</li>";
@@ -253,24 +252,18 @@
254253 $nt = $pair[0];
255254 $text = $pair[1]; # "text" means "caption" here
256255
 256+ $descQuery = false;
257257 if ( $nt->getNamespace() == NS_FILE ) {
258 - # Give extensions a chance to select the file revision for us
259 - $time = $sha1 = $descQuery = false;
260 - wfRunHooks( 'BeforeGalleryFindFile',
261 - array( &$this, &$nt, &$time, &$descQuery, &$sha1 ) );
262258 # Get the file...
263259 if ( $this->mParser instanceof Parser ) {
 260+ # Give extensions a chance to select the file revision for us
 261+ $time = $sha1 = false;
 262+ wfRunHooks( 'BeforeParserFetchFileAndTitle',
 263+ array( &$this->mParser, &$nt, &$time, &$sha1, &$descQuery ) );
264264 # Fetch and register the file (file title may be different via hooks)
265265 list( $img, $nt ) = $this->mParser->fetchFileAndTitle( $nt, $time, $sha1 );
266266 } else {
267 - if ( $time === '0' ) {
268 - $img = false; // broken thumbnail forced by hook
269 - } elseif ( $sha1 ) { // get by (sha1,timestamp)
270 - $img = RepoGroup::singleton()->findFileFromKey(
271 - $sha1, array( 'time' => $time ) );
272 - } else { // get by (name,timestamp)
273 - $img = wfFindFile( $nt, array( 'time' => $time ) );
274 - }
 267+ $img = wfFindFile( $nt );
275268 }
276269 } else {
277270 $img = false;
Index: trunk/extensions/FlaggedRevs/FlaggedRevs.php
@@ -436,8 +436,7 @@
437437 # Parser hooks, selects the desired images/templates
438438 $wgHooks['ParserClearState'][] = 'FlaggedRevsHooks::parserAddFields';
439439 $wgHooks['BeforeParserFetchTemplateAndtitle'][] = 'FlaggedRevsHooks::parserFetchStableTemplate';
440 -$wgHooks['BeforeParserMakeImageLinkObj'][] = 'FlaggedRevsHooks::parserFetchStableFile';
441 -$wgHooks['BeforeGalleryFindFile'][] = 'FlaggedRevsHooks::galleryFetchStableFile';
 440+$wgHooks['BeforeParserFetchFileAndTitle'][] = 'FlaggedRevsHooks::parserFetchStableFile';
442441 # ########
443442
444443 # ######## DB write operations #########
Index: trunk/extensions/FlaggedRevs/FlaggedRevs.hooks.php
@@ -329,12 +329,9 @@
330330 }
331331
332332 /**
333 - * (a) Select the desired images based on the selected stable version time/SHA-1
334 - * (b) Set specified versions in mImageTimeKeys
 333+ * Select the desired images based on the selected stable version time/SHA-1
335334 */
336 - public static function parserFetchStableFile(
337 - $parser, Title $nt, &$skip, &$time, &$query, &$sha1
338 - ) {
 335+ public static function parserFetchStableFile( $parser, Title $nt, &$time, &$sha1, &$query ) {
339336 if ( !( $parser instanceof Parser ) ) {
340337 return true; // nothing to do
341338 }
@@ -359,31 +356,8 @@
360357 }
361358
362359 /**
363 - * (a) Select the desired images based on the selected stable version time/SHA-1
364 - * (b) Set specified versions in mImageTimeKeys
 360+ * Select the desired images based on the selected stable version time/SHA-1
365361 */
366 - public static function galleryFetchStableFile( $ig, Title $nt, &$time, &$query, &$sha1 ) {
367 - $parser =& $ig->mParser; // convenience
368 - if ( !( $parser instanceof Parser ) || $nt->getNamespace() != NS_FILE ) {
369 - return true; // nothing to do
370 - }
371 - if ( !FRInclusionManager::singleton()->parserOutputIsStabilized() ) {
372 - return true; // trigger for stable version parsing only
373 - }
374 - # Get version, update mImageTimeKeys...
375 - list( $time, $sha1 ) = self::parserFindStableFile( $parser, $nt );
376 - # Stabilize the file link
377 - if ( $time ) {
378 - if ( $query != '' ) $query .= '&';
379 - $query = "filetimestamp=" . urlencode( wfTimestamp( TS_MW, $time ) );
380 - }
381 - return true;
382 - }
383 -
384 - /**
385 - * (a) Select the desired images based on the selected stable version time/SHA-1
386 - * (b) Set specified versions in mImageTimeKeys
387 - */
388362 protected static function parserFindStableFile( Parser $parser, Title $title ) {
389363 $time = false; // current version
390364 $sha1 = false; // corresponds to $time

Follow-up revisions

RevisionCommit summaryAuthorDate
r84669Follow-up r84659: no need to pass parser by reference to event handlersaaron08:44, 24 March 2011

Comments

#Comment by IAlex (talk | contribs)   08:07, 24 March 2011

$this should not be passed by reference to the hook.

Status & tagging log