r104733 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r104732‎ | r104733 | r104734 >
Date:20:11, 30 November 2011
Author:btongminh
Status:resolved (Comments)
Tags:
Comment:
Fix VipsScaler for Windows which does not support passing environment variables before the command. We have a nice wfShellExec() wrapper for that.
(bug 32720) Run the JavaScript after the entire page has loaded, including the images, by hooking into window.load() instead of document.ready()
Modified paths:
  • /trunk/extensions/VipsScaler/VipsScaler_body.php (modified) (history)
  • /trunk/extensions/VipsScaler/modules/ext.vipsScaler/ext.vipsScaler.js (modified) (history)

Diff [purge]

Index: trunk/extensions/VipsScaler/modules/ext.vipsScaler/ext.vipsScaler.js
@@ -1,4 +1,5 @@
2 -jQuery( function( $ ) {
 2+jQuery( window ).load( ( function( $ ) {
 3+ return function( event ) {
34 $.vipsScaler = {
45 /** function to alternate between both thumbnails */
56 switchThumbs: function() {
@@ -51,5 +52,6 @@
5253 function() { $.vipsScaler.switchThumbs(); }
5354 );
5455 }
55 -}
 56+};
 57+} )( jQuery )
5658 );
Index: trunk/extensions/VipsScaler/VipsScaler_body.php
@@ -400,9 +400,8 @@
401401 */
402402 public function execute() {
403403 # Build and escape the command string
404 - $cmd =
405 - 'IM_CONCURRENCY=1 ' .
406 - wfEscapeShellArg(
 404+ $env = array( 'IM_CONCURRENCY' => '1' );
 405+ $cmd = wfEscapeShellArg(
407406 $this->vips,
408407 array_shift( $this->args ),
409408 $this->input, $this->output
@@ -416,7 +415,7 @@
417416
418417 # Execute
419418 $retval = 0;
420 - $this->err = wfShellExec( $cmd, $retval );
 419+ $this->err = wfShellExec( $cmd, $retval, env );
421420
422421 # Cleanup temp file
423422 if ( $this->removeInput ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r104734Follow-up r104733, fix stupid missing $btongminh20:22, 30 November 2011
r104735Follow-up r104733, fix stupid missing $btongminh20:22, 30 November 2011
r104849MFT to 1.18wmf1 vipsTest issues...hashar13:06, 1 December 2011

Comments

#Comment by Hashar (talk | contribs)   13:15, 1 December 2011

being bold and removing tstarling keyword since I did the review instead.

Status & tagging log