r65157 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r65156‎ \| r65157 \| r65158 >
Date:	13:23, 17 April 2010
Author:	catrope
Status:	deferred
Tags:
Comment:	PagedTiffHandler: Initial cleanup. Latge-scale whitespace fixes upcoming. Adding FIXME comments and translating some comments from German and loading TiffReader using AutoLoader
Modified paths:	/trunk/extensions/PagedTiffHandler/PagedTiffHandler.php (modified) (history) /trunk/extensions/PagedTiffHandler/PagedTiffHandler_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/PagedTiffHandler/PagedTiffHandler_body.php
—	—	@@ -1,6 +1,6 @@
2	2	<?php
3	3	/**
4		~~- * Copyright (C) Wikimedia Deuschland, 2009~~
	4	+ * Copyright (C) Wikimedia Deutschland, 2009
5	5	* Authors Hallo Welt! Medienwerkstatt GmbH
6	6	* Authors Sebastian Ulbricht, Daniel Lynge, Marc Reymann, Markus Glaser
7	7	*
—	—	@@ -26,7 +26,8 @@
27	27	* Sets $wgShowEXIF to true when file is a tiff file.
28	28	* This does not influence other ImageHandlers, which are possibly dependent on read-exif-data.
29	29	*/
30		~~- function PagedTiffHandler() {~~
	30	+ function __construct() {
	31	+ // FIXME: Manipulating globals in a constructor this way is evil
31	32	global $wgShowEXIF;
32	33	$wgShowEXIF = true;
33	34	}
—	—	@@ -48,23 +49,23 @@
49	50	* Customize the thumbnail shell command.
50	51	*/
51	52	static function renderCommand( &$cmd, $srcPath, $dstPath, $page, $width, $height ) { ### why? ^DK
	53	+ // FIXME: This function seems useless
52	54	return true;
53	55	}
54	56
55	57	function isEnabled() { return true; }
56	58	function mustRender() { return true; }
57		~~- function isMultiPage($img = false) {~~
	59	+ function isMultiPage( $img = false ) {
58	60	if (!$img) return true;
59	61	$meta = unserialize($img->metadata);
60		~~- return ($meta['page_amount'] > 1);~~
	62	+ return $meta['page_amount'] > 1;
61	63	}
62	64
63	65	/**
64		~~- * various checks against the uploaded file~~
65		~~- * - maximum upload-size~~
	66	+ * Various checks against the uploaded file
	67	+ * - maximum upload size
66	68	* - maximum number of embedded files
67	69	* - maximum size of metadata
68		~~- * - maximum size of metadata~~
69	70	* - identify-errors
70	71	* - identify-warnings
71	72	* - check for running-identify-service
—	—	@@ -74,90 +75,87 @@
75	76	$wgTiffUseTiffReader, $wgTiffReaderPath, $wgTiffReaderCheckEofForJS;
76	77	wfLoadExtensionMessages( 'PagedTiffHandler' );
77	78	if($wgTiffUseTiffReader) {
78		~~- require_once($wgTiffReaderPath.'/TiffReader.php');~~
79	79	$tr = new TiffReader($tempName);
80	80	$tr->check();
81	81	if(!$tr->isValidTiff()) {
82	82	$error = 'tiff_bad_file';
83		~~- wfDebug( __METHOD__.": tiff_bad_file ($saveName)\n" );~~
	83	+ wfDebug( __METHOD__.": $error ($saveName)\n" );
84	84	return false;
85	85	}
86	86	if($tr->checkScriptAtEnd($wgTiffReaderCheckEofForJS)) {
87	87	$error = 'tiff_script_detected';
88		~~- wfDebug( __METHOD__.": tiff_script_detected ($saveName)\n" );~~
	88	+ wfDebug( __METHOD__.": $error ($saveName)\n" );
89	89	return false;
90	90	}
91	91	if(!$tr->checkSize()) {
92	92	$error = 'tiff_size_error';
93		~~- wfDebug( __METHOD__.": tiff_size_error ($saveName)\n" );~~
	93	+ wfDebug( __METHOD__.": $error ($saveName)\n" );
94	94	return false;
95	95	}
96	96	}
97	97	$meta = self::getTiffImage(false, $tempName)->retrieveMetaData();
98	98	if(!$meta && $meta != -1) {
99	99	$error = 'tiff_out_of_service';
100		~~- wfDebug( __METHOD__.": tiff_out_of_service ($saveName)\n" );~~
	100	+ wfDebug( __METHOD__.": $error ($saveName)\n" );
101	101	return false;
102	102	}
103	103	if($meta == -1) {
104	104	$error = 'tiff_error_cached';
105		~~- wfDebug( __METHOD__.": tiff_error_cached ($saveName)\n" );~~
	105	+ wfDebug( __METHOD__.": $error ($saveName)\n" );
106	106	}
107	107	return self::extCheck($meta, $error, $saveName);
108	108	}
109	109
110	110	function extCheck($meta, &$error, $saveName = '') {
111		~~- global $wgTiffMaxEmbedFiles, $wgTiffMaxMetaSize;~~
	111	+ global $wgTiffMaxEmbedFiles, $wgTiffMaxMetaSize;
112	112	if(isset($meta['errors'])) {
113	113	$error = 'tiff_bad_file';
114	114
115		~~- //NOTE: in future, it will become possible to pass parameters~~
	115	+ // NOTE: in future, it will become possible to pass parameters
116	116	//$error = array( 'tiff_bad_file' , join('<br />', $meta['errors']) );
117		~~- ### does that work now? ^DK~~
	117	+ // does that work now? ^DK
118	118
119		~~- wfDebug( __METHOD__.": tiff_bad_file ($saveName) " . join('; ', $meta['errors']) . "\n" );~~
	119	+ wfDebug( __METHOD__.": $error ($saveName) " . join('; ', $meta['errors']) . "\n" );
120	120	return false;
121	121	}
122	122	if((strlen(serialize($meta))+1) > $wgTiffMaxMetaSize) {
123	123	$error = 'tiff_too_much_meta';
124		~~- wfDebug( __METHOD__.": tiff_too_much_meta ($saveName)\n" );~~
	124	+ wfDebug( __METHOD__.": $error ($saveName)\n" );
125	125	return false;
126	126	}
127	127	if($wgTiffMaxEmbedFiles && $meta['page_amount'] > $wgTiffMaxEmbedFiles) {
128	128	$error = 'tiff_too_much_embed_files';
129		~~- wfDebug( __METHOD__.": tiff_too_much_embed_files ($saveName)\n" );~~
	129	+ wfDebug( __METHOD__.": $error ($saveName)\n" );
130	130	return false;
131	131	}
132	132	return true;
133	133	}
134	134
135	135	/**
136		~~- * maps MagicWord-IDs to parameters.~~
137		~~- * parameter 'lossy' was added.~~
	136	+ * Maps MagicWord-IDs to parameters.
	137	+ * Parameter 'lossy' was added.
138	138	*/
139	139	function getParamMap() {
	140	+ // FIXME: This function is unused and should probably be a static member anyway
140	141	return array(
141		~~- 'img_width' => 'width',~~
142		~~- // @todo check height ## MediaWiki bietet keine MagicWord-ID für height. Dies scheint ein Bug zu sein. ^SU~~
143		~~- 'img_page' => 'page',~~
144		~~- 'img_lossy' => 'lossy',~~
	142	+ 'img_width' => 'width',
	143	+ // @todo check height ## MediaWiki doesn't have a MagicWord-ID for height, appears to be a bug. ^SU
	144	+ 'img_page' => 'page',
	145	+ 'img_lossy' => 'lossy',
145	146	);
146	147	}
147	148
148	149
149		- /*
150		~~- * checks whether parameters are valid and have valid values.~~
151		~~- * check for lossy was added.~~
	150	+ /**
	151	+ * Checks whether parameters are valid and have valid values.
	152	+ * Check for lossy was added.
152	153	*/
153	154	function validateParam( $name, $value ) {
154	155	if ( in_array( $name, array( 'width', 'height', 'page', 'lossy' ) ) ) {
155	156	if($name == 'lossy') {
156		~~- if(in_array($value, array(1, 0, '1', '0', 'true', 'false', 'lossy', 'lossless'))) {~~
157		~~- return true;~~
158		~~- }~~
159		~~- return false;~~
	157	+ return in_array( $value, array( 1, 0, '1', '0', 'true', 'false', 'lossy', 'lossless' ) );
160	158	}
161		~~- if ( $value <= 0 \|\| $value > 65535 ) { ## ImageMagick läuft bei einer Seitenangabe höher als 65535 in einen Overflow. ^SU~~
	159	+ else if ( $value <= 0 \|\| $value > 65535 ) { // ImageMagick hits an overflow for values over 65536
162	160	return false;
163	161	} else {
164	162	return true;
—	—	@@ -168,20 +166,20 @@
169	167	}
170	168
171	169	/**
172		~~- * creates parameter string for file name.~~
173		~~- * page number was added.~~
	170	+ * Creates parameter string for file name.
	171	+ * Page number was added.
174	172	*/
175	173	function makeParamString( $params ) {
176	174	$page = isset( $params['page'] ) ? $params['page'] : 1;
177	175	if ( !isset( $params['width'] ) ) {
178	176	return false;
179	177	}
180		~~- $lossy = isset( $params['lossy'] ) && $params['lossy'] ? 'lossy' : 'lossless';~~
	178	+ $lossy = isset( $params['lossy'] ) && $params['lossy'] ? 'lossy' : 'lossless';
181	179	return "{$lossy}-page{$page}-{$params['width']}px";
182	180	}
183	181
184	182	/**
185		~~- * parses parameter string into an array.~~
	183	+ * Parses parameter string into an array.
186	184	*/
187	185	function parseParamString( $str ) {
188	186	$m = false;
—	—	@@ -193,24 +191,24 @@
194	192	}
195	193
196	194	/**
197		~~- * lossy-paramter added~~
	195	+ * Lossy parameter added
198	196	* TODO: The purpose of this function is not yet fully clear.
199	197	*/
200	198	function getScriptParams( $params ) { ## wtf?? ^DK
	199	+ // FIXME: This function is unused, seems to be useless,
	200	+ // and could be replaced with an array_intersect() call
201	201	return array(
202		~~- 'width' => $params['width'],~~
203		~~- 'page' => $params['page'],~~
204		~~- 'lossy' => $params['lossy'],~~
	202	+ 'width' => $params['width'],
	203	+ 'page' => $params['page'],
	204	+ 'lossy' => $params['lossy'],
205	205	);
206	206	}
207	207
208	208	/**
209		~~- * prepares param array and sets standard values~~
210		~~- * standard values for page and lossy are added~~
	209	+ * Prepares param array and sets standard values.
	210	+ * Standard values for page and lossy are added.
211	211	*/
212	212	function normaliseParams( $image, &$params ) {
213		~~- global $wgImageMagickIdentifyCommand;~~
214		-
215	213	$mimeType = $image->getMimeType();
216	214
217	215	if ( !isset( $params['width'] ) ) {
—	—	@@ -223,17 +221,16 @@
224	222	$params['page'] = $this->pageCount( $image );
225	223	}
226	224	if ( !isset( $params['lossy'] ) ) {
227		~~- $params['lossy'] = NULL;~~
	225	+ $params['lossy'] = null;
228	226	}
229		~~- $size = PagedTiffImage::getPageSize($this->getMetaArray($image), $params['page']);~~
	227	+ $size = PagedTiffImage::getPageSize($this->getMetaArray($image), $params['page']);
230	228	$srcWidth = $size['width'];
231	229	$srcHeight = $size['height'];
232	230
233	231	if ( isset( $params['height'] ) && $params['height'] != -1 ) {
234		~~- ## Wenn das Bild im Hochformat ist und der Height-Parameter angegeben ist,~~
235		~~- ## wird der Width-Parameter angepasst, sofern width x height die Original-Ratio~~
236		~~- ## zerstören würden. Dies dient der Paging-Vorschau auf der ImagePage,~~
237		~~- ## damit die Seiten-Vorschauen nicht das Seiten-Layout zerstören. ^SU~~
	232	+ // If the image is in letter format and the height parameter is set, the width
	233	+ // parameter is adjusted so the original ratio doesn't get messed up. This is
	234	+ // so the thumbnails on an ImagePage don't mess up the layout. ^SU
238	235	if ( $params['width'] * $srcHeight > $params['height'] * $srcWidth ) {
239	236	$params['width'] = wfFitBoxWidth( $srcWidth, $srcHeight, $params['height'] );
240	237	}
—	—	@@ -259,22 +256,22 @@
260	257	if ( !$metadata ) {
261	258	if($metadata == -1) {
262	259	return $this->doThumbError( @$params['width'], @$params['height'], 'tiff_error_cached' ); ##test this ^DK
263		~~- // $wgCacheType = CACHE_DB~~
	260	+ // $wgCacheType = CACHE_DB
264	261	}
265	262	return $this->doThumbError( @$params['width'], @$params['height'], 'tiff_no_metadata' );
266	263	}
267	264	if ( !$this->normaliseParams( $image, $params ) )
268	265	return new TransformParameterError( $params );
269	266
270		~~- ## get params an forces width, height and page to be integer~~
271		~~- $width = $params['width'] - 0;~~
272		~~- $height = $params['height'] - 0;~~
	267	+ // Get params and force width, height and page to be integers
	268	+ $width = intval( $params['width'] );
	269	+ $height = intval( $params['height'] );
273	270	$srcPath = $image->getPath();
274		~~- $page = $params['page'] - 0;~~
	271	+ $page = intval( $params['page'] );
275	272
276	273	$extension = $this->getThumbExtension($image, $page, $params['lossy']);
277		~~- $dstPath .= $extension;~~
278		~~- $dstUrl .= $extension;~~
	274	+ $dstPath .= $extension;
	275	+ $dstUrl .= $extension;
279	276
280	277	$meta = unserialize($metadata);
281	278
—	—	@@ -284,7 +281,7 @@
285	282
286	283	if ( is_file($dstPath) )
287	284	return new ThumbnailImage( $image, $dstUrl, $width,
288		~~- $height, $dstPath, $page );~~
	285	+ $height, $dstPath, $page );
289	286
290	287	if(isset($meta['page_data'][$page]['pixels']) && $meta['page_data'][$page]['pixels'] > $wgTiffMaxEmbedFileResolution)
291	288	return $this->doThumbError( $width, $height, 'tiff_sourcefile_too_large' );
—	—	@@ -319,9 +316,8 @@
320	317	$removed = $this->removeBadFile( $dstPath, $retval );
321	318
322	319	if ( $retval != 0 \|\| $removed ) {
323		~~- wfDebugLog( 'thumbnail',~~
324		~~- sprintf( 'thumbnail failed on %s: error %d "%s" from "%s"',~~
325		~~- wfHostname(), $retval, trim($err), $cmd ) );~~
	320	+ wfDebugLog( 'thumbnail', "thumbnail failed on " . wfHostname() .
	321	+ "; error $retval \"$err\" from \"$cmd\"" );
326	322	return new MediaTransformError( 'thumbnail_error', $width, $height, $err );
327	323	} else {
328	324	return new ThumbnailImage( $image, $dstUrl, $width, $height, $dstPath, $page );
—	—	@@ -330,12 +326,12 @@
331	327
332	328	/**
333	329	* Decides (taking lossy parameter into account) the filetype of the thumbnail.
334		~~- * If there is no lossy-Parameter (NULL = not set), the decision is being made~~
	330	+ * If there is no lossy parameter (null = not set), the decision is made
335	331	* according to the presence of an alpha value.
336	332	* (alpha == true = png, alpha == false = jpg)
337	333	*/
338	334	function getThumbExtension( $image, $page, $lossy ) {
339		~~- if($lossy === NULL) {~~
	335	+ if($lossy === null) {
340	336	$data = $this->getMetaArray($image);
341	337	if((strtolower($data['page_data'][$page]['alpha']) == 'true')) {
342	338	return '.png';
—	—	@@ -369,7 +365,7 @@
370	366	protected function doThumbError( $width, $height, $msg ) {
371	367	wfLoadExtensionMessages( 'PagedTiffHandler' );
372	368	return new MediaTransformError( 'thumbnail_error',
373		~~- $width, $height, wfMsg($msg) );~~
	369	+ $width, $height, wfMsg($msg) );
374	370	}
375	371
376	372	/**
—	—	@@ -519,12 +515,14 @@
520	516	}
521	517
522	518	/**
523		~~- * Returns a PagedTiffImage or create a new one if don´t exist.~~
	519	+ * Returns a PagedTiffImage or creates a new one if it doesn't exist.
	520	+ * @param Image $image The image object, or false if there isn't one
	521	+ * @param string $fileName The filename
524	522	*/
525	523	static function getTiffImage( $image, $path ) {
526		~~- ## Wenn kein ImageObject übergeben wurde, wird ein TiffImage-Object anhand des Pfades erzeugt.~~
527		~~- ## Ist ein ImageObject vorhanden, wird geprüft, ob darin schon eine TiffImage-Instanz gespeichert wurde.~~
528		~~- ## Ist dies nicht der Fall, wird eine Instanz erstellt und im ImageObject gespeichert. ^SU~~
	524	+ // If no Image object is passed, a TiffImage is created based on $path .
	525	+ // If there is an Image object, we check whether there's already a TiffImage instance in there;
	526	+ // if not, a new instance is created and stored in the Image object
529	527	if ( !$image )
530	528	$tiffimg = new PagedTiffImage( $path );
531	529	elseif ( !isset( $image->tiffImage ) )
Index: trunk/extensions/PagedTiffHandler/PagedTiffHandler.php
—	—	@@ -1,6 +1,6 @@
2	2	<?php
3	3	/**
4		~~- * Copyright (C) Wikimedia Deuschland, 2009~~
	4	+ * Copyright (C) Wikimedia Deutschland, 2009
5	5	* Authors Hallo Welt! Medienwerkstatt GmbH
6	6	* Authors Sebastian Ulbricht, Daniel Lynge, Marc Reymann, Markus Glaser
7	7	*
—	—	@@ -21,14 +21,13 @@
22	22	*
23	23	*/
24	24
25		~~-//error_reporting(E_ALL);~~
26	25	# Not a valid entry point, skip unless MEDIAWIKI is defined
27	26	if (!defined('MEDIAWIKI')) {
28	27	echo "PagedTiffHandler extension";
29	28	exit(1);
30	29	}
31	30
32		~~-/** Add to LocalSettings.php~~
	31	+/* Add to LocalSettings.php
33	32	require_once("$IP/extensions/PagedTiffHandler/PagedTiffHandler.php");
34	33
35	34	$wgUseImageMagick = true;
—	—	@@ -36,10 +35,8 @@
37	36	$wgImageMagickIdentifyCommand = "C:\Program Files\ImageMagick-6.5.6-Q8\identify";
38	37	$wgTiffExivCommand = "C:\Program Files\Exiv2\exiv2";
39	38	$wgMaxUploadSize = 1073741824;
	39	+*/
40	40
41		-
42		~~- */~~
43		-
44	41	$wgExtensionCredits['media'][] = array(
45	42	'path' => __FILE__,
46	43	'name' => 'PagedTiffHandler',
—	—	@@ -92,9 +89,9 @@
93	90	$wgTiffMaxEmbedFiles = 10000;
94	91	// Maximum resolution of embedded images (product of width x height pixels)
95	92	$wgTiffMaxEmbedFileResolution = 25600000; // max. Resolution 1600 x 1600 pixels
96		~~-// Maximum size of meta data~~
	93	+// Maximum size of metadata
97	94	$wgTiffMaxMetaSize = 67108864; // 64kB
98		~~-// TTL of Cacheentries for Errors~~
	95	+// TTL of cache entries for errors
99	96	$wgTiffErrorCacheTTL = 84600;
100	97
101	98	$wgFileExtensions[] = 'tiff';
—	—	@@ -102,11 +99,11 @@
103	100
104	101	$dir = dirname(__FILE__) . '/';
105	102	$wgExtensionMessagesFiles['PagedTiffHandler'] = $dir . 'PagedTiffHandler.i18n.php';
106		~~-$wgExtensionMessagesFiles['PagedTiffImage'] = $dir . 'PagedTiffHandler.i18n.php';~~
107	103	$wgAutoloadClasses['PagedTiffImage'] = $dir . 'PagedTiffHandler.image.php';
108	104	$wgAutoloadClasses['PagedTiffHandler'] = $dir . 'PagedTiffHandler_body.php';
	105	+$wgAutoloadClasses['TiffReader'] = $dir . 'TiffReader.php';
109	106	$wgMediaHandlers['image/tiff'] = 'PagedTiffHandler';
110	107	$wgHooks['UploadVerification'][] = 'PagedTiffHandler::check';
111	108	$wgHooks['LanguageGetMagic'][] = 'PagedTiffHandler::addTiffLossyMagicWordLang';
112	109	$wgHooks['PagedTiffHandlerRenderCommand'][] = 'PagedTiffHandler::renderCommand';
113		~~-$wgHooks['PagedTiffHandlerExivCommand'][] = 'PagedTiffImage::exivCommand';~~
\ No newline at end of file
	110	+$wgHooks['PagedTiffHandlerExivCommand'][] = 'PagedTiffImage::exivCommand';

Status & tagging log

01:38, 17 August 2010 Tim Starling (talk | contribs) changed the status of r65157 [removed: new added: deferred]