Make SVG files show "In other resolutions" at all sizes
authorBrian Wolff <bawolff+wn@gmail.com>
Thu, 22 May 2014 17:44:43 +0000 (14:44 -0300)
committerBrian Wolff <bawolff+wn@gmail.com>
Wed, 2 Jul 2014 18:01:00 +0000 (15:01 -0300)
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
tests/phpunit/includes/ImagePage404Test.php [new file with mode: 0644]
tests/phpunit/includes/ImagePageTest.php [new file with mode: 0644]
tests/phpunit/includes/media/MediaWikiMediaTestCase.php

index 60db202..89ca241 100644 (file)
@@ -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 (file)
index 0000000..3660456
--- /dev/null
@@ -0,0 +1,53 @@
+<?php
+/**
+ * For doing Image Page tests that rely on 404 thumb handling
+ */
+class ImagePage404Test extends MediaWikiMediaTestCase {
+
+       protected function getRepoOptions() {
+               return parent::getRepoOptions() + array( 'transformVia404' => 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 (file)
index 0000000..d5ecb95
--- /dev/null
@@ -0,0 +1,90 @@
+<?php
+class ImagePageTest extends MediaWikiMediaTestCase {
+
+       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 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 ),
+               );
+       }
+}
index 96347d9..7b64dfd 100644 (file)
@@ -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
-               ) );
+               );
        }
 
        /**