r83536 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r83535‎ \| r83536 \| r83537 >
Date:	18:12, 8 March 2011
Author:	mah
Status:	ok (Comments)
Tags:
Comment:	misc w/s cleanups, “svn diff -x-w” clean
Modified paths:	/trunk/phase3/includes/Article.php (modified) (history) /trunk/phase3/includes/Html.php (modified) (history) /trunk/phase3/includes/HttpFunctions.php (modified) (history) /trunk/phase3/includes/Import.php (modified) (history) /trunk/phase3/includes/User.php (modified) (history) /trunk/phase3/includes/WatchlistEditor.php (modified) (history) /trunk/phase3/includes/upload/UploadBase.php (modified) (history) /trunk/phase3/includes/upload/UploadStash.php (modified) (history) /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/upload/UploadBase.php
—	—	@@ -148,12 +148,12 @@
149	149	}
150	150
151	151	public function __construct() {}
152		-
	152	+
153	153	/**
154	154	* Returns the upload type. Should be overridden by child classes
155		- *
	155	+ *
156	156	* @since 1.18
157		~~- * @return string~~
	157	+ * @return string
158	158	*/
159	159	public function getSourceType() { return null; }
160	160
—	—	@@ -242,7 +242,7 @@
243	243	*/
244	244	$maxSize = self::getMaxUploadSize( $this->getSourceType() );
245	245	if( $this->mFileSize > $maxSize ) {
246		~~- return array(~~
	246	+ return array(
247	247	'status' => self::FILE_TOO_LARGE,
248	248	'max' => $maxSize,
249	249	);
—	—	@@ -350,9 +350,9 @@
351	351	protected function verifyFile() {
352	352	global $wgAllowJavaUploads;
353	353	# get the title, even though we are doing nothing with it, because
354		~~- # we need to populate mFinalExtension~~
	354	+ # we need to populate mFinalExtension
355	355	$this->getTitle();
356		-
	356	+
357	357	$this->mFileProps = File::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
358	358	$this->checkMacBinary();
359	359
—	—	@@ -373,11 +373,11 @@
374	374	}
375	375	}
376	376
377		~~- # Check for Java applets, which if uploaded can bypass cross-site~~
	377	+ # Check for Java applets, which if uploaded can bypass cross-site
378	378	# restrictions.
379	379	if ( !$wgAllowJavaUploads ) {
380	380	$this->mJavaDetected = false;
381		~~- $zipStatus = ZipDirectoryReader::read( $this->mTempPath,~~
	381	+ $zipStatus = ZipDirectoryReader::read( $this->mTempPath,
382	382	array( $this, 'zipEntryCallback' ) );
383	383	if ( !$zipStatus->isOK() ) {
384	384	$errors = $zipStatus->getErrorsArray();
—	—	@@ -422,16 +422,16 @@
423	423	$names = array( $entry['name'] );
424	424
425	425	// If there is a null character, cut off the name at it, because JDK's
426		~~- // ZIP_GetEntry() uses strcmp() if the name hashes match. If a file name~~
427		~~- // were constructed which had ".class\0" followed by a string chosen to~~
428		~~- // make the hash collide with the truncated name, that file could be~~
	426	+ // ZIP_GetEntry() uses strcmp() if the name hashes match. If a file name
	427	+ // were constructed which had ".class\0" followed by a string chosen to
	428	+ // make the hash collide with the truncated name, that file could be
429	429	// returned in response to a request for the .class file.
430	430	$nullPos = strpos( $entry['name'], "\000" );
431	431	if ( $nullPos !== false ) {
432	432	$names[] = substr( $entry['name'], 0, $nullPos );
433	433	}
434	434
435		~~- // If there is a trailing slash in the file name, we have to strip it,~~
	435	+ // If there is a trailing slash in the file name, we have to strip it,
436	436	// because that's what ZIP_GetEntry() does.
437	437	if ( preg_grep( '!\.class/?$!', $names ) ) {
438	438	$this->mJavaDetected = true;
—	—	@@ -470,12 +470,12 @@
471	471	$permErrors = array_merge( $permErrors, wfArrayDiff2( $permErrorsCreate, $permErrors ) );
472	472	return $permErrors;
473	473	}
474		-
	474	+
475	475	$overwriteError = $this->checkOverwrite( $user );
476	476	if ( $overwriteError !== true ) {
477	477	return array( array( $overwriteError ) );
478	478	}
479		-
	479	+
480	480	return true;
481	481	}
482	482
—	—	@@ -555,14 +555,14 @@
556	556	* @return Status indicating the whether the upload succeeded.
557	557	*/
558	558	public function performUpload( $comment, $pageText, $watch, $user ) {
559		~~- $status = $this->getLocalFile()->upload(~~
560		~~- $this->mTempPath,~~
561		~~- $comment,~~
	559	+ $status = $this->getLocalFile()->upload(
	560	+ $this->mTempPath,
	561	+ $comment,
562	562	$pageText,
563	563	File::DELETE_SOURCE,
564		~~- $this->mFileProps,~~
565		~~- false,~~
566		~~- $user~~
	564	+ $this->mFileProps,
	565	+ false,
	566	+ $user
567	567	);
568	568
569	569	if( $status->isGood() ) {
—	—	@@ -611,7 +611,7 @@
612	612	$this->mFinalExtension = trim( $ext[count( $ext ) - 1] );
613	613	} else {
614	614	$this->mFinalExtension = '';
615		-
	615	+
616	616	# No extension, try guessing one
617	617	$magic = MimeMagic::singleton();
618	618	$mime = $magic->guessMimeType( $this->mTempPath );
—	—	@@ -621,22 +621,22 @@
622	622	if ( $extList ) {
623	623	# Set the extension to the canonical extension
624	624	$this->mFinalExtension = strtok( $extList, ' ' );
625		-
	625	+
626	626	# Fix up the other variables
627	627	$this->mFilteredName .= ".{$this->mFinalExtension}";
628	628	$nt = Title::makeTitleSafe( NS_FILE, $this->mFilteredName );
629	629	$ext = array( $this->mFinalExtension );
630	630	}
631	631	}
632		-
	632	+
633	633	}
634	634
635	635	/* Don't allow users to override the blacklist (check file extension) */
636	636	global $wgCheckFileExtensions, $wgStrictFileExtensions;
637	637	global $wgFileExtensions, $wgFileBlacklist;
638		-
	638	+
639	639	$blackListedExtensions = $this->checkFileExtensionList( $ext, $wgFileBlacklist );
640		-
	640	+
641	641	if ( $this->mFinalExtension == '' ) {
642	642	$this->mTitleError = self::FILETYPE_MISSING;
643	643	return $this->mTitle = null;
—	—	@@ -703,15 +703,15 @@
704	704	*
705	705	* This method will stash a file in a temporary directory for later processing, and save the necessary descriptive info
706	706	* into the user's session.
707		~~- * This method returns the file object, which also has a 'sessionKey' property which can be passed through a form or~~
	707	+ * This method returns the file object, which also has a 'sessionKey' property which can be passed through a form or
708	708	* API request to find this stashed file again.
709	709	*
710	710	* @param $key String: (optional) the session key used to find the file info again. If not supplied, a key will be autogenerated.
711	711	* @return UploadStashFile stashed file
712	712	*/
713		~~- public function stashSessionFile( $key = null ) {~~
	713	+ public function stashSessionFile( $key = null ) {
714	714	$stash = RepoGroup::singleton()->getLocalRepo()->getUploadStash();
715		~~- $data = array(~~
	715	+ $data = array(
716	716	'mFileProps' => $this->mFileProps,
717	717	'mSourceType' => $this->getSourceType(),
718	718	);
—	—	@@ -1276,18 +1276,18 @@
1277	1277	}
1278	1278
1279	1279	/**
1280		~~- * Gets image info about the file just uploaded.~~
	1280	+ * Gets image info about the file just uploaded.
1281	1281	*
1282		~~- * Also has the effect of setting metadata to be an 'indexed tag name' in returned API result if~~
	1282	+ * Also has the effect of setting metadata to be an 'indexed tag name' in returned API result if
1283	1283	* 'metadata' was requested. Oddly, we have to pass the "result" object down just so it can do that
1284		~~- * with the appropriate format, presumably.~~
	1284	+ * with the appropriate format, presumably.
1285	1285	*
1286	1286	* @param $result ApiResult:
1287	1287	* @return Array: image info
1288	1288	*/
1289	1289	public function getImageInfo( $result ) {
1290	1290	$file = $this->getLocalFile();
1291		~~- // TODO This cries out for refactoring. We really want to say $file->getAllInfo(); here.~~
	1291	+ // TODO This cries out for refactoring. We really want to say $file->getAllInfo(); here.
1292	1292	// Perhaps "info" methods should be moved into files, and the API should just wrap them in queries.
1293	1293	if ( $file instanceof UploadStashFile ) {
1294	1294	$imParam = ApiQueryStashImageInfo::getPropertyNames();
—	—	@@ -1305,12 +1305,12 @@
1306	1306	unset( $code['status'] );
1307	1307	return Status::newFatal( $this->getVerificationErrorCode( $code ), $error );
1308	1308	}
1309		-
	1309	+
1310	1310	public static function getMaxUploadSize( $forType = null ) {
1311	1311	global $wgMaxUploadSize;
1312		-
	1312	+
1313	1313	if ( is_array( $wgMaxUploadSize ) ) {
1314		~~- if ( !is_null( $forType ) && isset( $wgMaxUploadSize[$forType] ) ) {~~
	1314	+ if ( !is_null( $forType) && isset( $wgMaxUploadSize[$forType] ) ) {
1315	1315	return $wgMaxUploadSize[$forType];
1316	1316	} else {
1317	1317	return $wgMaxUploadSize['*'];
—	—	@@ -1318,6 +1318,6 @@
1319	1319	} else {
1320	1320	return intval( $wgMaxUploadSize );
1321	1321	}
1322		-
	1322	+
1323	1323	}
1324	1324	}
Index: trunk/phase3/includes/upload/UploadStash.php
—	—	@@ -1,13 +1,13 @@
2	2	<?php
3		-/**
	3	+/**
4	4	* UploadStash is intended to accomplish a few things:
5	5	* - enable applications to temporarily stash files without publishing them to the wiki.
6	6	* - Several parts of MediaWiki do this in similar ways: UploadBase, UploadWizard, and FirefoggChunkedExtension
7	7	* And there are several that reimplement stashing from scratch, in idiosyncratic ways. The idea is to unify them all here.
8	8	* Mostly all of them are the same except for storing some custom fields, which we subsume into the data array.
9		~~- * - enable applications to find said files later, as long as the session or temp files haven't been purged.~~
	9	+ * - enable applications to find said files later, as long as the session or temp files haven't been purged.
10	10	* - enable the uploading user (and ONLY the uploading user) to access said files, and thumbnails of said files, via a URL.
11		~~- * We accomplish this by making the session serve as a URL->file mapping, on the assumption that nobody else can access~~
	11	+ * We accomplish this by making the session serve as a URL->file mapping, on the assumption that nobody else can access
12	12	* the session, even the uploading user. See SpecialUploadStash, which implements a web interface to some files stored this way.
13	13	*
14	14	*/
—	—	@@ -18,8 +18,8 @@
19	19
20	20	// repository that this uses to store temp files
21	21	// public because we sometimes need to get a LocalFile within the same repo.
22		~~- public $repo;~~
23		-
	22	+ public $repo;
	23	+
24	24	// array of initialized objects obtained from session (lazily initialized upon getFile())
25	25	private $files = array();
26	26
—	—	@@ -31,7 +31,7 @@
32	32	* Represents the session which contains temporarily stored files.
33	33	* Designed to be compatible with the session stashing code in UploadBase (should replace it eventually)
34	34	*/
35		~~- public function __construct( $repo ) {~~
	35	+ public function __construct( $repo ) {
36	36
37	37	// this might change based on wiki's configuration.
38	38	$this->repo = $repo;
—	—	@@ -43,7 +43,7 @@
44	44	if ( !isset( $_SESSION[UploadBase::SESSION_KEYNAME] ) ) {
45	45	$_SESSION[UploadBase::SESSION_KEYNAME] = array();
46	46	}
47		-
	47	+
48	48	}
49	49
50	50	/**
—	—	@@ -58,8 +58,8 @@
59	59	public function getFile( $key ) {
60	60	if ( ! preg_match( self::KEY_FORMAT_REGEX, $key ) ) {
61	61	throw new UploadStashBadPathException( "key '$key' is not in a proper format" );
62		~~- }~~
63		-
	62	+ }
	63	+
64	64	if ( !isset( $this->files[$key] ) ) {
65	65	if ( !isset( $_SESSION[UploadBase::SESSION_KEYNAME][$key] ) ) {
66	66	throw new UploadStashFileNotFoundException( "key '$key' not found in stash" );
—	—	@@ -70,7 +70,7 @@
71	71	if ($data['version'] !== UploadBase::SESSION_VERSION ) {
72	72	throw new UploadStashBadVersionException( $data['version'] . " does not match current version " . UploadBase::SESSION_VERSION );
73	73	}
74		-
	74	+
75	75	// separate the stashData into the path, and then the rest of the data
76	76	$path = $data['mTempPath'];
77	77	unset( $data['mTempPath'] );
—	—	@@ -102,7 +102,7 @@
103	103	wfDebug( "UploadStash: tried to stash file at '$path', but it doesn't exist\n" );
104	104	throw new UploadStashBadPathException( "path doesn't exist" );
105	105	}
106		~~- $fileProps = File::getPropsFromPath( $path );~~
	106	+ $fileProps = File::getPropsFromPath( $path );
107	107
108	108	// we will be initializing from some tmpnam files that don't have extensions.
109	109	// most of MediaWiki assumes all uploaded files have good extensions. So, we fix this.
—	—	@@ -113,7 +113,7 @@
114	114	throw new UploadStashFileException( "couldn't rename $path to have a better extension at $pathWithGoodExtension" );
115	115	}
116	116	$path = $pathWithGoodExtension;
117		~~- }~~
	117	+ }
118	118
119	119	// If no key was supplied, use content hash. Also has the nice property of collapsing multiple identical files
120	120	// uploaded this session, which could happen if uploads had failed.
—	—	@@ -123,7 +123,7 @@
124	124
125	125	if ( ! preg_match( self::KEY_FORMAT_REGEX, $key ) ) {
126	126	throw new UploadStashBadPathException( "key '$key' is not in a proper format" );
127		~~- }~~
	127	+ }
128	128
129	129	// if not already in a temporary area, put it there
130	130	$status = $this->repo->storeTemp( basename( $path ), $path );
—	—	@@ -148,7 +148,7 @@
149	149	// required info we always store. Must trump any other application info in $data
150	150	// 'mTempPath', 'mFileSize', and 'mFileProps' are arbitrary names
151	151	// chosen for compatibility with UploadBase's way of doing this.
152		~~- $requiredData = array(~~
	152	+ $requiredData = array(
153	153	'mTempPath' => $stashPath,
154	154	'mFileSize' => $fileProps['size'],
155	155	'mFileProps' => $fileProps,
—	—	@@ -159,7 +159,7 @@
160	160	// UploadWizard wants different things than say FirefoggChunkedUpload.)
161	161	wfDebug( __METHOD__ . " storing under $key\n" );
162	162	$_SESSION[UploadBase::SESSION_KEYNAME][$key] = array_merge( $data, $requiredData );
163		-
	163	+
164	164	return $this->getFile( $key );
165	165	}
166	166
—	—	@@ -193,12 +193,12 @@
194	194
195	195	/**
196	196	* Find or guess extension -- ensuring that our extension matches our mime type.
197		~~- * Since these files are constructed from php tempnames they may not start off~~
	197	+ * Since these files are constructed from php tempnames they may not start off
198	198	* with an extension.
199		~~- * XXX this is somewhat redundant with the checks that ApiUpload.php does with incoming~~
	199	+ * XXX this is somewhat redundant with the checks that ApiUpload.php does with incoming
200	200	* uploads versus the desired filename. Maybe we can get that passed to us...
201	201	*/
202		~~- public static function getExtensionForPath( $path ) {~~
	202	+ public static function getExtensionForPath( $path ) {
203	203	// Does this have an extension?
204	204	$n = strrpos( $path, '.' );
205	205	$extension = null;
—	—	@@ -209,8 +209,8 @@
210	210	$magic = MimeMagic::singleton();
211	211	$mimeType = $magic->guessMimeType( $path );
212	212	$extensions = explode( ' ', MimeMagic::singleton()->getExtensionsForType( $mimeType ) );
213		~~- if ( count( $extensions ) ) {~~
214		~~- $extension = $extensions[0];~~
	213	+ if ( count( $extensions ) ) {
	214	+ $extension = $extensions[0];
215	215	}
216	216	}
217	217
—	—	@@ -242,19 +242,19 @@
243	243	* @throws UploadStashBadPathException
244	244	* @throws UploadStashFileNotFoundException
245	245	*/
246		~~- public function __construct( $stash, $repo, $path, $key, $data ) {~~
	246	+ public function __construct( $stash, $repo, $path, $key, $data ) {
247	247	$this->sessionStash = $stash;
248	248	$this->sessionKey = $key;
249	249	$this->sessionData = $data;
250	250
251	251	// resolve mwrepo:// urls
252	252	if ( $repo->isVirtualUrl( $path ) ) {
253		~~- $path = $repo->resolveVirtualUrl( $path );~~
	253	+ $path = $repo->resolveVirtualUrl( $path );
254	254	}
255	255
256	256	// check if path appears to be sane, no parent traversals, and is in this repo's temp zone.
257	257	$repoTempPath = $repo->getZonePath( 'temp' );
258		~~- if ( ( ! $repo->validateFilename( $path ) ) \|\|~~
	258	+ if ( ( ! $repo->validateFilename( $path ) ) \|\|
259	259	( strpos( $path, $repoTempPath ) !== 0 ) ) {
260	260	wfDebug( "UploadStash: tried to construct an UploadStashFile from a file that should already exist at '$path', but path is not valid\n" );
261	261	throw new UploadStashBadPathException( 'path is not valid' );
—	—	@@ -285,13 +285,13 @@
286	286
287	287	/**
288	288	* Get the path for the thumbnail (actually any transformation of this file)
289		~~- * The actual argument is the result of thumbName although we seem to have~~
	289	+ * The actual argument is the result of thumbName although we seem to have
290	290	* buggy code elsewhere that expects a boolean 'suffix'
291	291	*
292	292	* @param $thumbName String: name of thumbnail (e.g. "120px-123456.jpg" ), or false to just get the path
293	293	* @return String: path thumbnail should take on filesystem, or containing directory if thumbname is false
294	294	*/
295		~~- public function getThumbPath( $thumbName = false ) {~~
	295	+ public function getThumbPath( $thumbName = false ) {
296	296	$path = dirname( $this->path );
297	297	if ( $thumbName !== false ) {
298	298	$path .= "/$thumbName";
—	—	@@ -300,8 +300,8 @@
301	301	}
302	302
303	303	/**
304		~~- * Return the file/url base name of a thumbnail with the specified parameters.~~
305		~~- * We override this because we want to use the pretty url name instead of the~~
	304	+ * Return the file/url base name of a thumbnail with the specified parameters.
	305	+ * We override this because we want to use the pretty url name instead of the
306	306	* ugly file name.
307	307	*
308	308	* @param $params Array: handler-specific parameters
—	—	@@ -314,33 +314,33 @@
315	315	/**
316	316	* Helper function -- given a 'subpage', return the local URL e.g. /wiki/Special:UploadStash/subpage
317	317	* @param {String} $subPage
318		~~- * @return {String} local URL for this subpage in the Special:UploadStash space.~~
	318	+ * @return {String} local URL for this subpage in the Special:UploadStash space.
319	319	*/
320	320	private function getSpecialUrl( $subPage ) {
321	321	return SpecialPage::getTitleFor( 'UploadStash', $subPage )->getLocalURL();
322	322	}
323	323
324		- /**
325		~~- * Get a URL to access the thumbnail~~
326		~~- * This is required because the model of how files work requires that~~
	324	+ /**
	325	+ * Get a URL to access the thumbnail
	326	+ * This is required because the model of how files work requires that
327	327	* the thumbnail urls be predictable. However, in our model the URL is not based on the filename
328	328	* (that's hidden in the session)
329	329	*
330	330	* @param $thumbName String: basename of thumbnail file -- however, we don't want to use the file exactly
331	331	* @return String: URL to access thumbnail, or URL with partial path
332	332	*/
333		~~- public function getThumbUrl( $thumbName = false ) {~~
	333	+ public function getThumbUrl( $thumbName = false ) {
334	334	wfDebug( __METHOD__ . " getting for $thumbName \n" );
335	335	return $this->getSpecialUrl( 'thumb/' . $this->getUrlName() . '/' . $thumbName );
336	336	}
337	337
338		- /**
	338	+ /**
339	339	* The basename for the URL, which we want to not be related to the filename.
340	340	* Will also be used as the lookup key for a thumbnail file.
341	341	*
342	342	* @return String: base url name, like '120px-123456.jpg'
343	343	*/
344		~~- public function getUrlName() {~~
	344	+ public function getUrlName() {
345	345	if ( ! $this->urlName ) {
346	346	$this->urlName = $this->sessionKey;
347	347	}
—	—	@@ -361,12 +361,12 @@
362	362	}
363	363
364	364	/**
365		~~- * Parent classes use this method, for no obvious reason, to return the path (relative to wiki root, I assume).~~
	365	+ * Parent classes use this method, for no obvious reason, to return the path (relative to wiki root, I assume).
366	366	* But with this class, the URL is unrelated to the path.
367	367	*
368	368	* @return String: url
369	369	*/
370		~~- public function getFullUrl() {~~
	370	+ public function getFullUrl() {
371	371	return $this->getUrl();
372	372	}
373	373
Index: trunk/phase3/includes/User.php
—	—	@@ -2831,7 +2831,7 @@
2832	2832	# Some wikis were converted from ISO 8859-1 to UTF-8, the passwords can't be converted
2833	2833	# Check for this with iconv
2834	2834	$cp1252Password = iconv( 'UTF-8', 'WINDOWS-1252//TRANSLIT', $password );
2835		~~- if ( $cp1252Password != $password &&~~
	2835	+ if ( $cp1252Password != $password &&
2836	2836	self::comparePasswords( $this->mPassword, $cp1252Password, $this->mId ) )
2837	2837	{
2838	2838	return true;
Index: trunk/phase3/includes/Article.php
—	—	@@ -1834,7 +1834,7 @@
1835	1835	if ( !$isRedirect && !is_null( $lastRevIsRedirect ) && $lastRevIsRedirect === $isRedirect ) {
1836	1836	return true;
1837	1837	}
1838		-
	1838	+
1839	1839	wfProfileIn( __METHOD__ );
1840	1840	if ( $isRedirect ) {
1841	1841	$this->insertRedirectEntry( $redirectTitle );
Index: trunk/phase3/includes/WatchlistEditor.php
—	—	@@ -60,7 +60,7 @@
61	61	} else {
62	62	$this->clearWatchlist( $user );
63	63	$user->invalidateCache();
64		~~- $output->addHTML( wfMsgExt( 'watchlistedit-raw-removed', 'parse',~~
	64	+ $output->addHTML( wfMsgExt( 'watchlistedit-raw-removed', 'parse',
65	65	$wgLang->formatNum( count( $current ) ) ) );
66	66	$this->showTitles( $current, $output, $wgUser->getSkin() );
67	67	}
Index: trunk/phase3/includes/Html.php
—	—	@@ -200,7 +200,7 @@
201	201	/**
202	202	* Returns "</$element>", except if $wgWellFormedXml is off, in which case
203	203	* it returns the empty string when that's guaranteed to be safe.
204		- *
	204	+ *
205	205	* @since 1.17
206	206	* @param $element string Name of the element, e.g., 'a'
207	207	* @return string A closing tag, if required
Index: trunk/phase3/includes/HttpFunctions.php
—	—	@@ -141,7 +141,7 @@
142	142	*/
143	143	class MWHttpRequest {
144	144	const SUPPORTS_FILE_POSTS = false;
145		-
	145	+
146	146	protected $content;
147	147	protected $timeout = 'default';
148	148	protected $headersOnly = null;
—	—	@@ -205,7 +205,7 @@
206	206
207	207	/**
208	208	* Generate a new request object
209		~~- * @param $url String: url to use~~
	209	+ * @param $url String: url to use
210	210	* @param $options Array: (optional) extra params to pass (see Http::request())
211	211	* @see MWHttpRequest::__construct
212	212	*/
—	—	@@ -565,7 +565,7 @@
566	566	*/
567	567	class CurlHttpRequest extends MWHttpRequest {
568	568	const SUPPORTS_FILE_POSTS = true;
569		-
	569	+
570	570	static $curlMessageMap = array(
571	571	6 => 'http-host-unreachable',
572	572	28 => 'http-timed-out'
—	—	@@ -692,7 +692,7 @@
693	693
694	694	if ( is_array( $this->postData ) ) {
695	695	$this->postData = wfArrayToCGI( $this->postData );
696		~~- }~~
	696	+ }
697	697
698	698	// At least on Centos 4.8 with PHP 5.1.6, using max_redirects to follow redirects
699	699	// causes a segfault
Index: trunk/phase3/includes/Import.php
—	—	@@ -192,7 +192,7 @@
193	193	* Dummy for now...
194	194	*/
195	195	public function importUpload( $revision ) {
196		~~- $revision->importUpload();~~
	196	+ $revision->importUpload();
197	197	//$dbw = wfGetDB( DB_MASTER );
198	198	//return $dbw->deadlockLoop( array( $revision, 'importUpload' ) );
199	199	return false;
—	—	@@ -604,12 +604,12 @@
605	605
606	606	private function processUpload( $pageInfo, $uploadInfo ) {
607	607	$revision = new WikiRevision;
608		~~- $text = isset( $uploadInfo['text'] ) ? $uploadInfo['text'] : '';~~
	608	+ $text = isset( $uploadInfo['text'] ) ? $uploadInfo['text'] : '';
609	609
610	610	$revision->setTitle( $pageInfo['_title'] );
611		~~- $revision->setID( $pageInfo['id'] );~~
	611	+ $revision->setID( $pageInfo['id'] );
612	612	$revision->setTimestamp( $uploadInfo['timestamp'] );
613		~~- $revision->setText( $text );~~
	613	+ $revision->setText( $text );
614	614	$revision->setFilename( $uploadInfo['filename'] );
615	615	$revision->setSrc( $uploadInfo['src'] );
616	616	$revision->setSize( intval( $uploadInfo['size'] ) );
—	—	@@ -622,7 +622,7 @@
623	623	$revision->setUserName( $uploadInfo['contributor']['username'] );
624	624	}
625	625
626		~~- return call_user_func( $this->mUploadCallback, $revision );~~
	626	+ return call_user_func( $this->mUploadCallback, $revision );
627	627	}
628	628
629	629	private function handleContributor() {
—	—	@@ -1057,7 +1057,7 @@
1058	1058	$resultDetails = array( 'internal' => $status->getWikiText() );
1059	1059	*/
1060	1060
1061		~~- // @todo Fixme: it may create a page without our desire, also wrong potentially.~~
	1061	+ // @todo Fixme: it may create a page without our desire, also wrong potentially.
1062	1062	// and, it will record a current upload, but we might want an archive version here
1063	1063
1064	1064	$file = wfLocalFile( $this->getTitle() );
—	—	@@ -1072,15 +1072,15 @@
1073	1073	return false;
1074	1074	}
1075	1075
1076		~~- $user = User::newFromName( $this->user_text );~~
	1076	+ $user = User::newFromName( $this->user_text );
1077	1077
1078	1078	$status = $file->upload( $source,
1079	1079	$this->getComment(),
1080	1080	$this->getComment(), // Initial page, if none present...
1081	1081	File::DELETE_SOURCE,
1082	1082	false, // props...
1083		~~- $this->getTimestamp(),~~
1084		~~- is_object( $user ) ? ( $user->isLoggedIn() ? $user : null ) : null );~~
	1083	+ $this->getTimestamp(),
	1084	+ is_object( $user ) ? ( $user->isLoggedIn() ? $user : null ) : null );
1085	1085
1086	1086	if( $status->isGood() ) {
1087	1087	// yay?
Index: trunk/phase3/languages/messages/MessagesEn.php
—	—	@@ -2963,10 +2963,10 @@
2964	2964	'undelete-show-file-submit' => 'Yes',
2965	2965
2966	2966	# Namespace form on various pages
2967		~~-'namespace' => 'Namespace:',~~
2968		~~-'invert' => 'Invert selection',~~
	2967	+'namespace' => 'Namespace:',
	2968	+'invert' => 'Invert selection',
2969	2969	'namespace_association' => 'Associated namespace',
2970		~~-'blanknamespace' => '(Main)',~~
	2970	+'blanknamespace' => '(Main)',
2971	2971
2972	2972	# Contributions
2973	2973	'contributions' => 'User contributions',

Follow-up revisions

Revision	Commit summary	Author	Date
r84265	Followup r83536 to address Jack Phoenix's comments about the bogus w/s “cle...	mah	20:58, 18 March 2011

Comments

#Comment by Jack Phoenix (talk | contribs) 19:24, 8 March 2011

Index: trunk/phase3/includes/upload/UploadBase.php
===================================================================
--- trunk/phase3/includes/upload/UploadBase.php	(revision 83535)
+++ trunk/phase3/includes/upload/UploadBase.php	(revision 83536)
[snip]
 		if ( is_array( $wgMaxUploadSize ) ) {
-			if ( !is_null( $forType ) && isset( $wgMaxUploadSize[$forType] ) ) {
+			if ( !is_null( $forType) && isset( $wgMaxUploadSize[$forType] ) ) {
 				return $wgMaxUploadSize[$forType];
 			} else {
 				return $wgMaxUploadSize['*'];

This is wrong, the spacing in the original version was correct.

Index: trunk/phase3/includes/upload/UploadStash.php
===================================================================
--- trunk/phase3/includes/upload/UploadStash.php	(revision 83535)
+++ trunk/phase3/includes/upload/UploadStash.php	(revision 83536)
[snip]
@@ -102,7 +102,7 @@
 			wfDebug( "UploadStash: tried to stash file at '$path', but it doesn't exist\n" );
 			throw new UploadStashBadPathException( "path doesn't exist" );
 		}
-                $fileProps = File::getPropsFromPath( $path );
+				$fileProps = File::getPropsFromPath( $path );
 
 		// we will be initializing from some tmpnam files that don't have extensions.
 		// most of MediaWiki assumes all uploaded files have good extensions. So, we fix this.

The change to tabs is good, but there seems to be two tabs too much.

Index: trunk/phase3/includes/HttpFunctions.php
===================================================================
--- trunk/phase3/includes/HttpFunctions.php	(revision 83535)
+++ trunk/phase3/includes/HttpFunctions.php	(revision 83536)
@@ -141,7 +141,7 @@
  */
 class MWHttpRequest {
 	const SUPPORTS_FILE_POSTS = false;
-	
+
 	protected $content;
 	protected $timeout = 'default';
 	protected $headersOnly = null;
@@ -205,7 +205,7 @@
 
 	/**
 	 * Generate a new request object
-        * @param $url String: url to use
+		* @param $url String: url to use
 	 * @param $options Array: (optional) extra params to pass (see Http::request())
 	 * @see MWHttpRequest::__construct
 	 */

The second tab should be just a space.

The Import.php changes also look a bit wonky, seems that the incorrect space indentation was not removed. For example:

-               $user = User::newFromName( $this->user_text );
+			   $user = User::newFromName( $this->user_text );

It should be indented with two tabs, but currently it is indented with three tabs and three spaces.

People can have different opinions about indentation in Messages*.php files, but IMO they need no indentation, just 'key' => 'value', is fine, so I'm going to say that those changes are OK.

#Comment by MarkAHershberger (talk | contribs) 00:11, 9 March 2011

thanks for looking over these changes!

Status & tagging log

13:10, 9 March 2011 Hashar (talk | contribs) changed the status of r83536 [removed: new added: ok]