r84278 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r84277‎ | r84278 | r84279 >
Date:22:28, 18 March 2011
Author:btongminh
Status:ok (Comments)
Tags:
Comment:
Added hook BitmapHandlerTransform to allow extension to transform a file without overriding the entire handler.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/media/Bitmap.php (modified) (history)

Diff [purge]

Index: trunk/phase3/docs/hooks.txt
@@ -578,6 +578,13 @@
579579 &$parser: Parser object
580580 &$ig: ImageGallery object
581581
 582+'BitmapHandlerTransform': before a file is transformed, gives extension the
 583+possibility to transform it themselves
 584+$handler: BitmapHandler
 585+$image: File
 586+&$scalerParams: Array with scaler parameters
 587+&$mto: null, set to a MediaTransformOutput
 588+
582589 'BlockIp': before an IP address or user is blocked
583590 $block: the Block object about to be saved
584591 $user: the user _doing_ the block (not the one being blocked)
Index: trunk/phase3/includes/media/Bitmap.php
@@ -101,6 +101,7 @@
102102 'mimeType' => $image->getMimeType(),
103103 'srcPath' => $image->getPath(),
104104 'dstPath' => $dstPath,
 105+ 'dstUrl' => $dstUrl,
105106 );
106107
107108 wfDebug( __METHOD__ . ": creating {$scalerParams['physicalDimensions']} thumbnail at $dstPath\n" );
@@ -135,8 +136,20 @@
136137 wfDebug( __METHOD__ . ": Unable to create thumbnail destination directory, falling back to client scaling\n" );
137138 return $this->getClientScalingThumbnailImage( $image, $scalerParams );
138139 }
139 -
 140+
 141+ # Try a hook
 142+ $mto = null;
 143+ wfRunHooks( 'BitmapHandlerTransform', array( $this, $image, &$scalerParams, &$mto ) );
 144+ if ( !is_null( $mto ) ) {
 145+ wfDebug( __METHOD__ . ": Hook to BitmapHandlerTransform created an mto\n" );
 146+ $scaler = 'hookaborted';
 147+ }
 148+
140149 switch ( $scaler ) {
 150+ case 'hookaborted':
 151+ # Handled by the hook above
 152+ $err = $mto->isError() ? $mto : false;
 153+ break;
141154 case 'im':
142155 $err = $this->transformImageMagick( $image, $scalerParams );
143156 break;
@@ -161,6 +174,8 @@
162175 # Thumbnail was zero-byte and had to be removed
163176 return new MediaTransformError( 'thumbnail_error',
164177 $scalerParams['clientWidth'], $scalerParams['clientHeight'] );
 178+ } elseif ( $mto ) {
 179+ return $mto;
165180 } else {
166181 return new ThumbnailImage( $image, $dstUrl, $scalerParams['clientWidth'],
167182 $scalerParams['clientHeight'], $dstPath );
@@ -443,7 +458,7 @@
444459 * @param $errMsg string Error message
445460 * @return MediaTransformError
446461 */
447 - protected function getMediaTransformError( $params, $errMsg ) {
 462+ public function getMediaTransformError( $params, $errMsg ) {
448463 return new MediaTransformError( 'thumbnail_error', $params['clientWidth'],
449464 $params['clientHeight'], $errMsg );
450465 }
Index: trunk/phase3/RELEASE-NOTES
@@ -116,6 +116,8 @@
117117 file description page. The sizes are set by $wgImageLimits.
118118 * (bug 28031) Add pageCount support to ArchivedFile
119119 * (bug 27924) PhpHttpRequest doesn't return response body if HTTP != 200
 120+* Added hook BitmapHandlerTransform to allow extension to transform a file
 121+ without overriding the entire handler.
120122
121123 === Bug fixes in 1.18 ===
122124 * (bug 23119) WikiError class and subclasses are now marked as deprecated

Comments

#Comment by Nikerabbit (talk | contribs)   09:23, 19 March 2011

Does this mean that the class has too long functions which makes extending the class too annoying?

#Comment by Bryan (talk | contribs)   15:40, 20 March 2011

Extending is sometimes not possible. For example, I want to have a scaler that scales standard bitmap images, but PNGs are handled by subclass PNGHandler, GIFs by GIFHandler, etc. Then adding a hook is the simplest solution rather than writing three subclasses which in someway have to interact with a common scaler backend.

#Comment by Reedy (talk | contribs)   22:43, 10 August 2011
+			wfDebug( __METHOD__ . ": Hook to BitmapHandlerTransform created an mto\n" );

"created an mto" doesn't seem very clear to me.

Is it supposed to be "created an $mto"? Or is their something that

Minor issue, otherwise ok.

#Comment by Bryan (talk | contribs)   12:11, 12 August 2011

mto=MediaTransformOutput. Perhaps better is "Hook to BitmapHandlerTransform created a thumbnail"

Status & tagging log