From 3de0b70abe0c87d8f796da6700fe33bf6d43cbd8 Mon Sep 17 00:00:00 2001 From: Brian Wolff Date: Thu, 22 May 2014 14:44:43 -0300 Subject: [PATCH] Make SVG files show "In other resolutions" at all sizes Previously it only showed the sizes smaller than the nominal size of the svg, which is silly for an infinitely scalabe vector image. This splits up the function a bit, in order to be able to do unit testing. This also changes the link below SVGs to always be "Show original file" (show-big-image), which I think makes more sense anyhow. Bug: 6834 Bug: 36911 Change-Id: Ic18e555f16940c658842148c155771ef31ac5db9 --- includes/page/ImagePage.php | 130 ++++++++++++------ tests/phpunit/includes/ImagePage404Test.php | 53 +++++++ tests/phpunit/includes/ImagePageTest.php | 90 ++++++++++++ .../includes/media/MediaWikiMediaTestCase.php | 11 +- 4 files changed, 239 insertions(+), 45 deletions(-) create mode 100644 tests/phpunit/includes/ImagePage404Test.php create mode 100644 tests/phpunit/includes/ImagePageTest.php diff --git a/includes/page/ImagePage.php b/includes/page/ImagePage.php index 60db2025c2..89ca241662 100644 --- a/includes/page/ImagePage.php +++ b/includes/page/ImagePage.php @@ -296,7 +296,7 @@ class ImagePage extends Article { } protected function openShowImage() { - global $wgImageLimits, $wgEnableUploads, $wgSend404Code; + global $wgEnableUploads, $wgSend404Code; $this->loadFile(); $out = $this->getContext()->getOutput(); @@ -341,49 +341,17 @@ class ImagePage extends Article { if ( $this->displayImg->allowInlineDisplay() ) { # image - # "Download high res version" link below the image # $msgsize = wfMessage( 'file-info-size', $width_orig, $height_orig, # Linker::formatSize( $this->displayImg->getSize() ), $mime )->escaped(); # We'll show a thumbnail of this image - if ( $width > $maxWidth || $height > $maxHeight ) { - # Calculate the thumbnail size. - # First case, the limiting factor is the width, not the height. - /** @todo // FIXME: Possible division by 0. bug 36911 */ - if ( $width / $height >= $maxWidth / $maxHeight ) { - /** @todo // FIXME: Possible division by 0. bug 36911 */ - $height = round( $height * $maxWidth / $width ); - $width = $maxWidth; - # Note that $height <= $maxHeight now. - } else { - /** @todo // FIXME: Possible division by 0. bug 36911 */ - $newwidth = floor( $width * $maxHeight / $height ); - /** @todo // FIXME: Possible division by 0. bug 36911 */ - $height = round( $height * $newwidth / $width ); - $width = $newwidth; - # Note that $height <= $maxHeight now, but might not be identical - # because of rounding. - } + if ( $width > $maxWidth || $height > $maxHeight || $this->displayImg->isVectorized() ) { + list( $width, $height ) = $this->getDisplayWidthHeight( + $maxWidth, $maxHeight, $width, $height + ); $linktext = wfMessage( 'show-big-image' )->escaped(); - if ( $this->displayImg->getRepo()->canTransformVia404() ) { - $thumbSizes = $wgImageLimits; - // Also include the full sized resolution in the list, so - // that users know they can get it. This will link to the - // original file asset if mustRender() === false. In the case - // that we mustRender, some users have indicated that they would - // find it useful to have the full size image in the rendered - // image format. - $thumbSizes[] = array( $width_orig, $height_orig ); - } else { - # Creating thumb links triggers thumbnail generation. - # Just generate the thumb for the current users prefs. - $thumbSizes = array( $this->getImageLimitsFromOption( $user, 'thumbsize' ) ); - if ( !$this->displayImg->mustRender() ) { - // We can safely include a link to the "full-size" preview, - // without actually rendering. - $thumbSizes[] = array( $width_orig, $height_orig ); - } - } + + $thumbSizes = $this->getThumbSizes( $width, $height, $width_orig, $height_orig ); # Generate thumbnails or thumbnail links as needed... $otherSizes = array(); foreach ( $thumbSizes as $size ) { @@ -393,7 +361,10 @@ class ImagePage extends Article { // the current thumbnail's size ($width/$height) // since that is added to the message separately, so // it can be denoted as the current size being shown. - if ( $size[0] <= $width_orig && $size[1] <= $height_orig + // Vectorized images are "infinitely" big, so all thumb + // sizes are shown. + if ( ( ($size[0] <= $width_orig && $size[1] <= $height_orig) + || $this->displayImg->isVectorized() ) && $size[0] != $width && $size[1] != $height ) { $sizeLink = $this->makeSizeLink( $params, $size[0], $size[1] ); @@ -403,6 +374,7 @@ class ImagePage extends Article { } } $otherSizes = array_unique( $otherSizes ); + $msgsmall = ''; $sizeLinkBigImagePreview = $this->makeSizeLink( $params, $width, $height ); if ( $sizeLinkBigImagePreview ) { @@ -421,9 +393,6 @@ class ImagePage extends Article { # Some sort of audio file that doesn't have dimensions # Don't output a no hi res message for such a file $msgsmall = ''; - } elseif ( $this->displayImg->isVectorized() ) { - # For vectorized images, full size is just the frame size - $msgsmall = ''; } else { # Image is small enough to show full size on image page $msgsmall = wfMessage( 'file-nohires' )->parse(); @@ -1084,6 +1053,81 @@ EOT ); return $langSelectLine; } + + /** + * Get the width and height to display image at. + * + * @note This method assumes that it is only called if one + * of the dimensions are bigger than the max, or if the + * image is vectorized. + * + * @param $maxWidth integer Max width to display at + * @param $maxHeight integer Max height to display at + * @param $width integer Actual width of the image + * @param $height integer Actual height of the image + * @throws MWException + * @return Array (width, height) + */ + protected function getDisplayWidthHeight( $maxWidth, $maxHeight, $width, $height ) { + if ( !$maxWidth || !$maxHeight ) { + // should never happen + throw new MWException( 'Using a choice from $wgImageLimits that is 0x0' ); + } + + if ( !$width || !$height ) { + return array( 0, 0 ); + } + + # Calculate the thumbnail size. + if ( $width <= $maxWidth && $height <= $maxHeight ) { + // Vectorized image, do nothing. + } elseif ( $width / $height >= $maxWidth / $maxHeight ) { + # The limiting factor is the width, not the height. + $height = round( $height * $maxWidth / $width ); + $width = $maxWidth; + # Note that $height <= $maxHeight now. + } else { + $newwidth = floor( $width * $maxHeight / $height ); + $height = round( $height * $newwidth / $width ); + $width = $newwidth; + # Note that $height <= $maxHeight now, but might not be identical + # because of rounding. + } + return array( $width, $height ); + } + + /** + * Get alternative thumbnail sizes. + * + * @note This will only list several alternatives if thumbnails are rendered on 404 + * @param $origWidth Actual width of image + * @param $origHeight Actual height of image + * @return Array An array of [width, height] pairs. + */ + protected function getThumbSizes( $origWidth, $origHeight ) { + global $wgImageLimits; + if ( $this->displayImg->getRepo()->canTransformVia404() ) { + $thumbSizes = $wgImageLimits; + // Also include the full sized resolution in the list, so + // that users know they can get it. This will link to the + // original file asset if mustRender() === false. In the case + // that we mustRender, some users have indicated that they would + // find it useful to have the full size image in the rendered + // image format. + $thumbSizes[] = array( $origWidth, $origHeight ); + } else { + # Creating thumb links triggers thumbnail generation. + # Just generate the thumb for the current users prefs. + $thumbSizes = array( $this->getImageLimitsFromOption( $this->getContext()->getUser(), 'thumbsize' ) ); + if ( !$this->displayImg->mustRender() ) { + // We can safely include a link to the "full-size" preview, + // without actually rendering. + $thumbSizes[] = array( $origWidth, $origHeight ); + } + } + return $thumbSizes; + } + } /** diff --git a/tests/phpunit/includes/ImagePage404Test.php b/tests/phpunit/includes/ImagePage404Test.php new file mode 100644 index 0000000000..3660456902 --- /dev/null +++ b/tests/phpunit/includes/ImagePage404Test.php @@ -0,0 +1,53 @@ + true ); + } + + function setUp() { + $this->setMwGlobals( 'wgImageLimits', array( + array( 320, 240 ), + array( 640, 480 ), + array( 800, 600 ), + array( 1024, 768 ), + array( 1280, 1024 ) + ) ); + parent::setUp(); + } + + function getImagePage( $filename ) { + $title = Title::makeTitleSafe( NS_FILE, $filename ); + $file = $this->dataFile( $filename ); + $iPage = new ImagePage( $title ); + $iPage->setFile( $file ); + return $iPage; + } + + /** + * @dataProvider providerGetThumbSizes + * @param $filename String + * @param $expectedNumberThumbs integer How many thumbnails to show + */ + function testGetThumbSizes( $filename, $expectedNumberThumbs ) { + $iPage = $this->getImagePage( $filename ); + $reflection = new ReflectionClass( $iPage ); + $reflMethod = $reflection->getMethod( 'getThumbSizes' ); + $reflMethod->setAccessible( true ); + + $actual = $reflMethod->invoke( $iPage, 545, 700 ); + $this->assertEquals( count( $actual ), $expectedNumberThumbs ); + } + + function providerGetThumbSizes() { + return array( + array( 'animated.gif', 6 ), + array( 'Toll_Texas_1.svg', 6 ), + array( '80x60-Greyscale.xcf', 6 ), + array( 'jpeg-comment-binary.jpg', 6 ), + ); + } +} diff --git a/tests/phpunit/includes/ImagePageTest.php b/tests/phpunit/includes/ImagePageTest.php new file mode 100644 index 0000000000..d5ecb9570d --- /dev/null +++ b/tests/phpunit/includes/ImagePageTest.php @@ -0,0 +1,90 @@ +setMwGlobals( 'wgImageLimits', array( + array( 320, 240 ), + array( 640, 480 ), + array( 800, 600 ), + array( 1024, 768 ), + array( 1280, 1024 ) + ) ); + parent::setUp(); + } + + function getImagePage( $filename ) { + $title = Title::makeTitleSafe( NS_FILE, $filename ); + $file = $this->dataFile( $filename ); + $iPage = new ImagePage( $title ); + $iPage->setFile( $file ); + return $iPage; + } + + /** + * @dataProvider providerGetDisplayWidthHeight + * @param $dimensions Array [maxWidth, maxHeight, width, height] + * @param $expected Array [width, height] The width and height we expect to display at + */ + function testGetDisplayWidthHeight( $dim, $expected ) { + $iPage = $this->getImagePage( 'animated.gif' ); + $reflection = new ReflectionClass( $iPage ); + $reflMethod = $reflection->getMethod( 'getDisplayWidthHeight' ); + $reflMethod->setAccessible( true ); + + $actual = $reflMethod->invoke( $iPage, $dim[0], $dim[1], $dim[2], $dim[3] ); + $this->assertEquals( $actual, $expected ); + } + + function providerGetDisplayWidthHeight() { + return array( + array( + array( 1024.0, 768.0, 600.0, 600.0 ), + array( 600.0, 600.0 ) + ), + array( + array( 1024.0, 768.0, 1600.0, 600.0 ), + array( 1024.0, 384.0 ) + ), + array( + array( 1024.0, 768.0, 1024.0, 768.0 ), + array( 1024.0, 768.0 ) + ), + array( + array( 1024.0, 768.0, 800.0, 1000.0 ), + array( 614.0, 768.0 ) + ), + array( + array( 1024.0, 768.0, 0, 1000 ), + array( 0, 0 ) + ), + array( + array( 1024.0, 768.0, 2000, 0 ), + array( 0, 0 ) + ), + ); + } + + /** + * @dataProvider providerGetThumbSizes + * @param $filename String + * @param $expectedNumberThumbs integer How many thumbnails to show + */ + function testGetThumbSizes( $filename, $expectedNumberThumbs ) { + $iPage = $this->getImagePage( $filename ); + $reflection = new ReflectionClass( $iPage ); + $reflMethod = $reflection->getMethod( 'getThumbSizes' ); + $reflMethod->setAccessible( true ); + + $actual = $reflMethod->invoke( $iPage, 545, 700 ); + $this->assertEquals( count( $actual ), $expectedNumberThumbs ); + } + + function providerGetThumbSizes() { + return array( + array( 'animated.gif', 2 ), + array( 'Toll_Texas_1.svg', 1 ), + array( '80x60-Greyscale.xcf', 1 ), + array( 'jpeg-comment-binary.jpg', 2 ), + ); + } +} diff --git a/tests/phpunit/includes/media/MediaWikiMediaTestCase.php b/tests/phpunit/includes/media/MediaWikiMediaTestCase.php index 96347d9400..7b64dfde24 100644 --- a/tests/phpunit/includes/media/MediaWikiMediaTestCase.php +++ b/tests/phpunit/includes/media/MediaWikiMediaTestCase.php @@ -29,11 +29,18 @@ abstract class MediaWikiMediaTestCase extends MediaWikiTestCase { 'wikiId' => wfWikiId(), 'containerPaths' => $containers ) ); - $this->repo = new FSRepo( array( + $this->repo = new FSRepo( $this->getRepoOptions() ); + } + + /** + * @return Array Argument for FSRepo constructor + */ + protected function getRepoOptions() { + return array( 'name' => 'temp', 'url' => 'http://localhost/thumbtest', 'backend' => $this->backend - ) ); + ); } /** -- 2.20.1