From: jarry1250 Date: Fri, 24 May 2013 12:56:06 +0000 (+0200) Subject: The language used to render SVGs should be definable X-Git-Tag: 1.31.0-rc.0~19485 X-Git-Url: http://git.cyclocoop.org/%24self?a=commitdiff_plain;h=3f5d41d4135740573f642b843a675f7daf5a2644;p=lhc%2Fweb%2Fwiklou.git The language used to render SVGs should be definable See bug for context. The implementation is slightly untidy because I've written it so as to avoid invalidating the existing SVG thumbs -- there will be no immediate difference (visual/performance/other) as a result of this. Tested by me in both... * [[File:Example.svg|thumb|lang=fr]] AND * http://example.org/w/index.php?title=File:Example.svg&lang=fr ...modes. Example file on https://commons.wikimedia.org/wiki/File:Gerrit_patchset_25838_test.svg Added parser tests. Bug: 32987 Change-Id: I4cadf96ecd5e169a88ad468a0478d355db980103 --- diff --git a/includes/ImagePage.php b/includes/ImagePage.php index a5beb5d983..8cfbc7383d 100644 --- a/includes/ImagePage.php +++ b/includes/ImagePage.php @@ -311,6 +311,12 @@ class ImagePage extends Article { } else { $params = array( 'page' => $page ); } + + $renderLang = $request->getVal( 'lang' ); + if ( !is_null( $renderLang ) ) { + $params['lang'] = $renderLang; + } + $width_orig = $this->displayImg->getWidth( $page ); $width = $width_orig; $height_orig = $this->displayImg->getHeight( $page ); diff --git a/includes/media/MediaTransformOutput.php b/includes/media/MediaTransformOutput.php index 1d197b1d58..86323993fd 100644 --- a/includes/media/MediaTransformOutput.php +++ b/includes/media/MediaTransformOutput.php @@ -32,7 +32,7 @@ abstract class MediaTransformOutput { */ var $file; - var $width, $height, $url, $page, $path; + var $width, $height, $url, $page, $path, $lang; /** * @var array Associative array mapping optional supplementary image files @@ -197,17 +197,26 @@ abstract class MediaTransformOutput { /** * @param $title string - * @param $params array + * @param $params string|array Query parameters to add * @return array */ - public function getDescLinkAttribs( $title = null, $params = '' ) { - $query = ''; + public function getDescLinkAttribs( $title = null, $params = array() ) { + if ( is_array( $params ) ) { + $query = $params; + } else { + $query = array(); + } if ( $this->page && $this->page !== 1 ) { - $query = 'page=' . urlencode( $this->page ); + $query['page'] = $this->page; } - if ( $params ) { - $query .= $query ? '&' . $params : $params; + if( $this->lang ) { + $query['lang'] = $this->lang; } + + if ( is_string( $params ) && $params !== '' ) { + $query = $params . '&' . wfArrayToCgi( $query ); + } + $attribs = array( 'href' => $this->file->getTitle()->getLocalURL( $query ), 'class' => 'image', @@ -242,10 +251,12 @@ class ThumbnailImage extends MediaTransformOutput { # Previous parameters: # $file, $url, $width, $height, $path = false, $page = false + $defaults = array( + 'page' => false, + 'lang' => false + ); + if ( is_array( $parameters ) ) { - $defaults = array( - 'page' => false - ); $actualParams = $parameters + $defaults; } else { # Using old format, should convert. Later a warning could be added here. @@ -254,7 +265,7 @@ class ThumbnailImage extends MediaTransformOutput { 'width' => $path, 'height' => $parameters, 'page' => ( $numArgs > 5 ) ? func_get_arg( 5 ) : false - ); + ) + $defaults; $path = ( $numArgs > 4 ) ? func_get_arg( 4 ) : false; } @@ -269,6 +280,7 @@ class ThumbnailImage extends MediaTransformOutput { $this->height = round( $actualParams['height'] ); $this->page = $actualParams['page']; + $this->lang = $actualParams['lang']; } /** diff --git a/includes/media/SVG.php b/includes/media/SVG.php index a133f6f856..f0356f51fd 100644 --- a/includes/media/SVG.php +++ b/includes/media/SVG.php @@ -115,6 +115,7 @@ class SvgHandler extends ImageHandler { $clientHeight = $params['height']; $physicalWidth = $params['physicalWidth']; $physicalHeight = $params['physicalHeight']; + $lang = isset( $params['lang'] ) ? $params['lang'] : 'en'; if ( $flags & self::TRANSFORM_LATER ) { return new ThumbnailImage( $image, $dstUrl, $dstPath, $params ); @@ -132,7 +133,7 @@ class SvgHandler extends ImageHandler { } $srcPath = $image->getLocalRefPath(); - $status = $this->rasterize( $srcPath, $dstPath, $physicalWidth, $physicalHeight ); + $status = $this->rasterize( $srcPath, $dstPath, $physicalWidth, $physicalHeight, $lang ); if ( $status === true ) { return new ThumbnailImage( $image, $dstUrl, $dstPath, $params ); } else { @@ -147,10 +148,11 @@ class SvgHandler extends ImageHandler { * @param string $dstPath * @param string $width * @param string $height + * @param string $lang Language code of the language to render the SVG in * @throws MWException * @return bool|MediaTransformError */ - public function rasterize( $srcPath, $dstPath, $width, $height ) { + public function rasterize( $srcPath, $dstPath, $width, $height, $lang = false ) { global $wgSVGConverters, $wgSVGConverter, $wgSVGConverterPath; $err = false; $retval = ''; @@ -158,7 +160,7 @@ class SvgHandler extends ImageHandler { if ( is_array( $wgSVGConverters[$wgSVGConverter] ) ) { // This is a PHP callable $func = $wgSVGConverters[$wgSVGConverter][0]; - $args = array_merge( array( $srcPath, $dstPath, $width, $height ), + $args = array_merge( array( $srcPath, $dstPath, $width, $height, $lang ), array_slice( $wgSVGConverters[$wgSVGConverter], 1 ) ); if ( !is_callable( $func ) ) { throw new MWException( "$func is not callable" ); @@ -176,9 +178,15 @@ class SvgHandler extends ImageHandler { wfEscapeShellArg( $dstPath ) ), $wgSVGConverters[$wgSVGConverter] ) . " 2>&1"; + + $env = array(); + if( $lang !== false ) { + $env['LANG'] = $lang; + } + wfProfileIn( 'rsvg' ); wfDebug( __METHOD__ . ": $cmd\n" ); - $err = wfShellExec( $cmd, $retval ); + $err = wfShellExec( $cmd, $retval, $env ); wfProfileOut( 'rsvg' ); } } @@ -357,4 +365,68 @@ class SvgHandler extends ImageHandler { } return $result; } + + + /** + * @param string $name Parameter name + * @param $string $value Parameter value + * @return bool Validity + */ + function validateParam( $name, $value ) { + if ( in_array( $name, array( 'width', 'height' ) ) ) { + // Reject negative heights, widths + return ( $value > 0 ); + } elseif( $name == 'lang' ) { + // Validate $code + if( !Language::isValidBuiltinCode( $value ) ) { + wfDebug( "Invalid user language code\n" ); + return false; + } + return true; + } + // Only lang, width and height are acceptable keys + return false; + } + + /** + * @param array $params name=>value pairs of parameters + * @return string Filename to use + */ + function makeParamString( $params ) { + $lang = ''; + if( isset( $params['lang'] ) && $params['lang'] !== 'en' ) { + $params['lang'] = mb_strtolower( $params['lang'] ); + $lang = "lang{$params['lang']}-"; + } + if ( !isset( $params['width'] ) ) { + return false; + } + return "$lang{$params['width']}px"; + } + + function parseParamString( $str ) { + $m = false; + if ( preg_match( '/^lang([a-z]+(?:-[a-z]+)*)-(\d+)px$/', $str, $m ) ) { + return array( 'width' => array_pop( $m ), 'lang' => $m[1] ); + } elseif( preg_match( '/^(\d+)px$/', $str, $m ) ) { + return array( 'width' => $m[1], 'lang' => 'en' ); + } else { + return false; + } + } + + function getParamMap() { + return array( 'img_lang' => 'lang', 'img_width' => 'width' ); + } + + /** + * @param $params + * @return array + */ + function getScriptParams( $params ) { + return array( + 'width' => $params['width'], + 'lang' => $params['lang'], + ); + } } diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index ee4a5333ce..c539dbaa8d 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -274,6 +274,7 @@ $magicWords = array( 'img_center' => array( 1, 'center', 'centre' ), 'img_framed' => array( 1, 'framed', 'enframed', 'frame' ), 'img_frameless' => array( 1, 'frameless' ), + 'img_lang' => array( 1, 'lang=$1' ), 'img_page' => array( 1, 'page=$1', 'page $1' ), 'img_upright' => array( 1, 'upright', 'upright=$1', 'upright $1' ), 'img_border' => array( 1, 'border' ), diff --git a/tests/TestsAutoLoader.php b/tests/TestsAutoLoader.php index 9f8887b596..1e9c2af986 100644 --- a/tests/TestsAutoLoader.php +++ b/tests/TestsAutoLoader.php @@ -85,6 +85,8 @@ $wgAutoloadClasses += array( 'MockFSFile' => "$testDir/phpunit/mocks/filebackend/MockFSFile.php", 'MockFileBackend' => "$testDir/phpunit/mocks/filebackend/MockFileBackend.php", 'MockBitmapHandler' => "$testDir/phpunit/mocks/media/MockBitmapHandler.php", + 'MockImageHandler' => "$testDir/phpunit/mocks/media/MockBitmapHandler.php", + 'MockSvgHandler' => "$testDir/phpunit/mocks/media/MockBitmapHandler.php", # tests/phpunit/languages 'LanguageClassesTestCase' => "$testDir/phpunit/languages/LanguageClassesTestCase.php", diff --git a/tests/parser/parserTest.inc b/tests/parser/parserTest.inc index 972ad8f2c8..1c3ba1dcaf 100644 --- a/tests/parser/parserTest.inc +++ b/tests/parser/parserTest.inc @@ -1,6 +1,8 @@ * http://www.mediawiki.org/ @@ -681,6 +683,8 @@ class ParserTest { 'wgNoFollowDomainExceptions' => array(), 'wgThumbnailScriptPath' => false, 'wgUseImageResize' => true, + 'wgSVGConverter' => 'null', + 'wgSVGConverters' => array( 'null' => 'echo "1">$output' ), 'wgLocaltimezone' => 'UTC', 'wgAllowExternalImages' => true, 'wgUseTidy' => false, @@ -869,6 +873,8 @@ class ParserTest { # Clear the message cache MessageCache::singleton()->clear(); + // Remember to update newParserTests.php after changing the below + // (and it uses a slightly different syntax just for teh lulz) $this->uploadDir = $this->setupUploadDir(); $user = User::createNew( 'WikiSysop' ); $image = wfLocalFile( Title::makeTitle( NS_FILE, 'Foobar.jpg' ) ); @@ -902,6 +908,19 @@ class ParserTest { 'fileExists' => true ), $this->db->timestamp( '20130225203040' ), $user ); + $image = wfLocalFile( Title::makeTitle( NS_FILE, 'Foobar.svg' ) ); + $image->recordUpload2( '', 'Upload of some lame SVG', 'Some lame SVG', array( + 'size' => 12345, + 'width' => 240, + 'height' => 180, + 'bits' => 24, + 'media_type' => MEDIATYPE_DRAWING, + 'mime' => 'image/svg+xml', + 'metadata' => serialize( array() ), + 'sha1' => wfBaseConvert( '', 16, 36, 31 ), + 'fileExists' => true + ), $this->db->timestamp( '20010115123500' ), $user ); + # This image will be blacklisted in [[MediaWiki:Bad image list]] $image = wfLocalFile( Title::makeTitle( NS_FILE, 'Bad.jpg' ) ); $image->recordUpload2( '', 'zomgnotcensored', 'Borderline image', array( @@ -985,7 +1004,11 @@ class ParserTest { copy( "$IP/skins/monobook/wiki.png", "$dir/e/ea/Thumb.png" ); wfMkdirParents( $dir . '/0/09', null, __METHOD__ ); copy( "$IP/skins/monobook/headbg.jpg", "$dir/0/09/Bad.jpg" ); - + wfMkdirParents( $dir . '/f/ff', null, __METHOD__ ); + copy( "$IP/skins/monobook/headbg.jpg", "$dir/f/ff/Foobar.svg" ); + file_put_contents( "$dir/f/ff/Foobar.svg", + '' . + '' ); return $dir; } @@ -1035,6 +1058,14 @@ class ParserTest { "$dir/0/09/Bad.jpg", + "$dir/f/ff/Foobar.svg", + "$dir/thumb/f/ff/Foobar.svg/180px-Foobar.svg.png", + "$dir/thumb/f/ff/Foobar.svg/360px-Foobar.svg.png", + "$dir/thumb/f/ff/Foobar.svg/langde-270px-Foobar.svg.png", + "$dir/thumb/f/ff/Foobar.svg/270px-Foobar.svg.png", + "$dir/thumb/f/ff/Foobar.svg/langde-180px-Foobar.svg.png", + "$dir/thumb/f/ff/Foobar.svg/langde-360px-Foobar.svg.png", + "$dir/math/f/a/5/fa50b8b616463173474302ca3e63586b.png", ) ); @@ -1048,10 +1079,13 @@ class ParserTest { "$dir/thumb/3/3a/Foobar.jpg", "$dir/thumb/3/3a", "$dir/thumb/3", - "$dir/e/ea", "$dir/e", - + "$dir/f/ff/", + "$dir/f/", + "$dir/thumb/f/ff/Foobar.svg", + "$dir/thumb/f/ff/", + "$dir/thumb/f/", "$dir/0/09/", "$dir/0/", "$dir/thumb", diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt index f7296bdb69..4aadd7aed8 100644 --- a/tests/parser/parserTests.txt +++ b/tests/parser/parserTests.txt @@ -5216,7 +5216,7 @@ Magic Word: {{NUMBEROFFILES}} !! input {{NUMBEROFFILES}} !! result -

3 +

4

!! end @@ -7757,6 +7757,37 @@ Thumbnail image caption with a free URL and explicit alt !! end + +!! test +SVG thumbnails with no language set +!! options +!! input +[[File:Foobar.svg|thumb|width=200]] +!! result +
width=200
+ +!! end + +!! test +SVG thumbnails with language de +!! options +!! input +[[File:Foobar.svg|thumb|width=200|lang=de]] +!! result +
width=200
+ +!! end + +!! test +SVG thumbnails with invalid language code +!! options +!! input +[[File:Foobar.svg|thumb|width=200|lang=invalid.language.code]] +!! result +
lang=invalid.language.code
+ +!! end + !! test BUG 1887: A ISBN with a thumbnail !! input @@ -13965,6 +13996,7 @@ HttP://MediaWiki.Org/

!! end + ### ### Parsoids-specific tests ### Parsoid-PHP parser incompatibilities diff --git a/tests/phpunit/includes/parser/NewParserTest.php b/tests/phpunit/includes/parser/NewParserTest.php index 8cee07bad6..6975939a75 100644 --- a/tests/phpunit/includes/parser/NewParserTest.php +++ b/tests/phpunit/includes/parser/NewParserTest.php @@ -102,6 +102,10 @@ class NewParserTest extends MediaWikiTestCase { $tmpGlobals['wgParser'] = new StubObject( 'wgParser', $GLOBALS['wgParserConf']['class'], array( $GLOBALS['wgParserConf'] ) ); + $tmpGlobals['wgFileExtensions'][] = 'svg'; + $tmpGlobals['wgSVGConverter'] = 'rsvg'; + $tmpGlobals['wgSVGConverters']['rsvg'] = '$path/rsvg-convert -w $width -h $height $input -o $output'; + if ( $GLOBALS['wgStyleDirectory'] === false ) { $tmpGlobals['wgStyleDirectory'] = "$IP/skins"; } @@ -113,6 +117,8 @@ class NewParserTest extends MediaWikiTestCase { foreach ( $wgMediaHandlers as $type => $handler ) { $tmpGlobals['wgMediaHandlers'][$type] = 'MockBitmapHandler'; } + // Vector images have to be handled slightly differently + $tmpGlobals['wgMediaHandlers']['image/svg+xml'] = 'MockSvgHandler'; $tmpHooks = $wgHooks; $tmpHooks['ParserTestParser'][] = 'ParserTestParserHook::setup'; @@ -270,6 +276,20 @@ class NewParserTest extends MediaWikiTestCase { $this->db->timestamp( '20010115123500' ), $user ); } + $image = wfLocalFile( Title::makeTitle( NS_FILE, 'Foobar.svg' ) ); + if ( !$this->db->selectField( 'image', '1', array( 'img_name' => $image->getName() ) ) ) { + $image->recordUpload2( '', 'Upload of some lame SVG', 'Some lame SVG', array( + 'size' => 12345, + 'width' => 200, + 'height' => 200, + 'bits' => 24, + 'media_type' => MEDIATYPE_DRAWING, + 'mime' => 'image/svg+xml', + 'metadata' => serialize( array() ), + 'sha1' => wfBaseConvert( '', 16, 36, 31 ), + 'fileExists' => true + ), $this->db->timestamp( '20010115123500' ), $user ); + } } //ParserTest setup/teardown functions @@ -440,6 +460,19 @@ class NewParserTest extends MediaWikiTestCase { $backend->store( array( 'src' => "$IP/skins/monobook/headbg.jpg", 'dst' => "$base/local-public/0/09/Bad.jpg" ) ); + + // No helpful SVG file to copy, so make one ourselves + $tmpDir = wfTempDir(); + $tempFsFile = new TempFSFile( "$tmpDir/Foobar.svg" ); + $tempFsFile->autocollect(); // destroy file when $tempFsFile leaves scope + file_put_contents( "$tmpDir/Foobar.svg", + '' . + 'Foo' ); + + $backend->prepare( array( 'dir' => "$base/local-public/f/ff" ) ); + $backend->quickStore( array( + 'src' => "$tmpDir/Foobar.svg", 'dst' => "$base/local-public/f/ff/Foobar.svg" + ) ); } /** @@ -492,6 +525,14 @@ class NewParserTest extends MediaWikiTestCase { "$base/local-public/0/09/Bad.jpg", + "$base/local-public/f/ff/Foobar.svg", + "$base/local-thumb/f/ff/Foobar.svg/180px-Foobar.svg.jpg", + "$base/local-thumb/f/ff/Foobar.svg/270px-Foobar.svg.jpg", + "$base/local-thumb/f/ff/Foobar.svg/360px-Foobar.svg.jpg", + "$base/local-thumb/f/ff/Foobar.svg/langde-180px-Foobar.svg.jpg", + "$base/local-thumb/f/ff/Foobar.svg/langde-270px-Foobar.svg.jpg", + "$base/local-thumb/f/ff/Foobar.svg/langde-360px-Foobar.svg.jpg", + "$base/local-public/math/f/a/5/fa50b8b616463173474302ca3e63586b.png", ) ); diff --git a/tests/phpunit/mocks/media/MockBitmapHandler.php b/tests/phpunit/mocks/media/MockBitmapHandler.php index 2ae30a342c..742b41e4dd 100644 --- a/tests/phpunit/mocks/media/MockBitmapHandler.php +++ b/tests/phpunit/mocks/media/MockBitmapHandler.php @@ -21,6 +21,19 @@ * @ingroup Media */ +class MockBitmapHandler extends BitmapHandler { + function doTransform( $image, $dstPath, $dstUrl, $params, $flags = 0 ) { + return MockImageHandler::doFakeTransform( $this, $image, $dstPath, $dstUrl, $params, $flags ); + } + function doClientImage( $image, $scalerParams ) { + return $this->getClientScalingThumbnailImage( $image, $scalerParams ); + } +} +class MockSvgHandler extends SvgHandler { + function doTransform( $image, $dstPath, $dstUrl, $params, $flags = 0 ) { + return MockImageHandler::doFakeTransform( $this, $image, $dstPath, $dstUrl, $params, $flags ); + } +} /** * Mock handler for images. * @@ -28,7 +41,7 @@ * * @ingroup Media */ -class MockBitmapHandler extends BitmapHandler { +class MockImageHandler { /** * Override BitmapHandler::doTransform() making sure we do not generate @@ -36,14 +49,14 @@ class MockBitmapHandler extends BitmapHandler { * will be consumed by the unit test. There is no need to create a real * thumbnail on the filesystem. */ - function doTransform( $image, $dstPath, $dstUrl, $params, $flags = 0 ) { + static function doFakeTransform( $that, $image, $dstPath, $dstUrl, $params, $flags = 0 ) { # Example of what we receive: # $image: LocalFile # $dstPath: /tmp/transform_7d0a7a2f1a09-1.jpg # $dstUrl : http://example.com/images/thumb/0/09/Bad.jpg/320px-Bad.jpg # $params: width: 320, descriptionUrl http://trunk.dev/wiki/File:Bad.jpg - $this->normaliseParams( $image, $params ); + $that->normaliseParams( $image, $params ); $scalerParams = array( # The size to which the image will be resized @@ -67,9 +80,11 @@ class MockBitmapHandler extends BitmapHandler { # In some cases, we do not bother generating a thumbnail. if ( !$image->mustRender() && $scalerParams['physicalWidth'] == $scalerParams['srcWidth'] - && $scalerParams['physicalHeight'] == $scalerParams['srcHeight'] ) { + && $scalerParams['physicalHeight'] == $scalerParams['srcHeight'] + ) { wfDebug( __METHOD__ . ": returning unscaled image\n" ); - return $this->getClientScalingThumbnailImage( $image, $scalerParams ); + // getClientScalingThumbnailImage is protected + return $that->doClientImage( $image, $scalerParams ); } return new ThumbnailImage( $image, $dstUrl, false, $params );