Use square bounding boxes for default-sized thumbnails

Thumbnails for portrait-orientation images have always been "too big",
especially when displayed in a gallery.  The 'upright' option did not
completely fix the issue.  Using a square bounding box for thumbnails
(and 'framed' images) without an explicit size specifiction provides
a better default appearance.

This also provides a clean syntax for content authored using
Parsoid/Visual Editor, which prefers square bounding boxes.

See:
https://www.mediawiki.org/wiki/Requests_for_comment/Square_bounding_boxes

Bug: 63903
Change-Id: I665d8945843d3b5437a74e376b63c44965590116

diff --git a/RELEASE-NOTES-1.24 b/RELEASE-NOTES-1.24
index 439420b..f067e32 100644 (file)
--- a/RELEASE-NOTES-1.24
+++ b/RELEASE-NOTES-1.24
@@ -38,6 +38,9 @@ production.
 * WikitextContent will now render redirects with the expected "redirect"
   header, rather than as an ordered list. Code calling Article::viewRedirect
   can probably be changed to no longer special-case redirects.
+* (bug 63903) Thumbnails without an explicit size specification are
+  now resized to a square bounding box. This gives better results for
+  non-landscape thumbnails.
 
 === Bug fixes in 1.24 ===
 * (bug 49116) Footer copyright notice is now always displayed in user language

diff --git a/includes/Linker.php b/includes/Linker.php
index d0759ed..7724548 100644 (file)
--- a/includes/Linker.php
+++ b/includes/Linker.php
@@ -603,6 +603,7 @@ class Linker {
                                }
 
                                // Reduce width for upright images when parameter 'upright' is used
+                               $useSquare = !isset( $fp['upright'] );
                                if ( isset( $fp['upright'] ) && $fp['upright'] == 0 ) {
                                        $fp['upright'] = $wgThumbUpright;
                                }
@@ -614,11 +615,16 @@ class Linker {
                                        round( $wgThumbLimits[$widthOption] * $fp['upright'], -1 ) :
                                        $wgThumbLimits[$widthOption];
 
-                               // Use width which is smaller: real image width or user preference width
+                               // Use whichever is smaller: real image width or user preference width
                                // Unless image is scalable vector.
                                if ( !isset( $hp['height'] ) && ( $hp['width'] <= 0 ||
-                                               $prefWidth < $hp['width'] || $file->isVectorized() ) ) {
+                                               $prefWidth < $hp['width'] ||
+                                               ( $useSquare && $prefWidth < $file->getHeight( $page ) ) ||
+                                               $file->isVectorized() ) ) {
                                        $hp['width'] = $prefWidth;
+                                       if ( $useSquare ) {
+                                               $hp['height'] = $prefWidth;
+                                       }
                                }
                        }
                }

diff --git a/tests/parser/parserTest.inc b/tests/parser/parserTest.inc
index 1875ac5..0bbee17 100644 (file)
--- a/tests/parser/parserTest.inc
+++ b/tests/parser/parserTest.inc
@@ -1015,6 +1015,34 @@ class ParserTest {
                                'fileExists'  => true
                ), $this->db->timestamp( '20010115123500' ), $user );
 
+               $image = wfLocalFile( Title::makeTitle( NS_FILE, 'Portrait.png' ) );
+               # again, note that size/width/height below are ignored; see above.
+               $image->recordUpload2( '', 'Upload of tall bitmap', 'Some tall bitmap', array(
+                       'size'        => 12345,
+                       'width'       => 180,
+                       'height'      => 240,
+                       'bits'        => 8,
+                       'media_type'  => MEDIATYPE_BITMAP,
+                       'mime'        => 'image/png',
+                       'metadata'    => serialize( array() ),
+                       'sha1'        => wfBaseConvert( '', 16, 36, 31 ),
+                       'fileExists'  => true
+               ), $this->db->timestamp( '20140515134200' ), $user );
+
+               $image = wfLocalFile( Title::makeTitle( NS_FILE, 'Portrait.svg' ) );
+               # again, note that size/width/height below are ignored; see above.
+               $image->recordUpload2( '', 'Upload of tall SVG', 'Some tall SVG', array(
+                       'size'        => 12345,
+                       'width'       => 180,
+                       'height'      => 240,
+                       'bits'        => 24,
+                       'media_type'  => MEDIATYPE_DRAWING,
+                       'mime'        => 'image/svg+xml',
+                       'metadata'    => serialize( array() ),
+                       'sha1'        => wfBaseConvert( '', 16, 36, 31 ),
+                       'fileExists'  => true
+               ), $this->db->timestamp( '20140325124200' ), $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(
@@ -1144,6 +1172,13 @@ class ParserTest {
                        '<?xml version="1.0" encoding="utf-8"?>' .
                        '<svg xmlns="http://www.w3.org/2000/svg"' .
                        ' version="1.1" width="240" height="180"/>' );
+               wfMkdirParents( $dir . '/9/93', null, __METHOD__ );
+               copy( "$IP/tests/phpunit/data/media/Portrait.png", "$dir/9/93/Portrait.png" );
+               wfMkdirParents( $dir . '/f/fc', null, __METHOD__ );
+               file_put_contents( "$dir/f/fc/Portrait.svg",
+                       '<?xml version="1.0" encoding="utf-8"?>' .
+                       '<svg xmlns="http://www.w3.org/2000/svg"' .
+                       ' version="1.1" width="180" height="240"/>' );
                wfMkdirParents( $dir . '/5/5f', null, __METHOD__ );
                copy( "$IP/tests/phpunit/data/media/LoremIpsum.djvu", "$dir/5/5f/LoremIpsum.djvu" );
 
@@ -1230,6 +1265,17 @@ class ParserTest {
                                "$dir/thumb/f/ff/Foobar.svg/langde-270px-Foobar.svg.png",
                                "$dir/thumb/f/ff/Foobar.svg/langde-360px-Foobar.svg.png",
 
+                               "$dir/9/93/Portrait.png",
+                               "$dir/thumb/9/93/Portrait.png/165px-Portrait.png",
+
+                               "$dir/f/fc/Portrait.svg",
+                               "$dir/thumb/f/fc/Portrait.svg/165px-Portrait.svg.png",
+                               "$dir/thumb/f/fc/Portrait.svg/225px-Portrait.svg.png",
+                               "$dir/thumb/f/fc/Portrait.svg/247px-Portrait.svg.png",
+                               "$dir/thumb/f/fc/Portrait.svg/330px-Portrait.svg.png",
+                               "$dir/thumb/f/fc/Portrait.svg/337px-Portrait.svg.png",
+                               "$dir/thumb/f/fc/Portrait.svg/450px-Portrait.svg.png",
+
                                "$dir/math/f/a/5/fa50b8b616463173474302ca3e63586b.png",
                        )
                );
@@ -1241,10 +1287,18 @@ class ParserTest {
                                "$dir/thumb/3/3a/Foobar.jpg",
                                "$dir/thumb/3/3a",
                                "$dir/thumb/3",
+                               "$dir/9/93",
+                               "$dir/9",
+                               "$dir/thumb/9/93/Portrait.png",
+                               "$dir/thumb/9/93/",
+                               "$dir/thumb/9/",
                                "$dir/e/ea",
                                "$dir/e",
+                               "$dir/f/fc/",
                                "$dir/f/ff/",
                                "$dir/f/",
+                               "$dir/thumb/f/fc/Portrait.svg",
+                               "$dir/thumb/f/fc/",
                                "$dir/thumb/f/ff/Foobar.svg",
                                "$dir/thumb/f/ff/",
                                "$dir/thumb/f/",

diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt
index 569c165..55801aa 100644 (file)
--- a/tests/parser/parserTests.txt
+++ b/tests/parser/parserTests.txt
@@ -7755,7 +7755,7 @@ Magic Word: {{NUMBEROFFILES}}
 !! wikitext
 {{NUMBEROFFILES}}
 !! html
-<p>5
+<p>7
 </p>
 !! end
 
@@ -10793,12 +10793,16 @@ parsoid=wt2html,wt2wt,html2html
 # Image sizing.
 # See https://www.mediawiki.org/wiki/Help:Images#Size_and_frame
 # and https://bugzilla.wikimedia.org/show_bug.cgi?id=62258
-# Foobar has actual size of 1941x220
+# Foobar.jpg has actual size of 1941x220
+# Portrait.svg has actual size of 180x240
+# Portrait.png has actual size of 180x240
 # 1. Thumbs & frameless always reduce, can't be enlarged unless it's
 #    a scalable format.
 # 2. Framed images always ignore size options; always render at default size.
 # 3. "Unspecified format" and border are the only types which can be
 #    enlarged.
+# 4. Without an explicit size specification, thumbnails are
+#    resized to a square bounding box.
 
 !! test
 Image: "unspecified format" and border enlarge
@@ -10916,6 +10920,55 @@ parsoid=wt2html,wt2wt,html2html
 <figure class="mw-default-size" typeof="mw:Image/Frame"><a href="File:Foobar.jpg"><img resource="./File:Foobar.jpg" src="//example.com/images/3/3a/Foobar.jpg" height="220" width="1941"/></a></figure><figure typeof="mw:Image/Frame"><a href="File:Foobar.jpg"><img resource="./File:Foobar.jpg" src="//example.com/images/3/3a/Foobar.jpg" height="220" width="1941"/></a></figure><figure typeof="mw:Image/Frame"><a href="File:Foobar.jpg"><img resource="./File:Foobar.jpg" src="//example.com/images/3/3a/Foobar.jpg" height="220" width="1941"/></a></figure><figure typeof="mw:Image/Frame"><a href="File:Foobar.jpg"><img resource="./File:Foobar.jpg" src="//example.com/images/3/3a/Foobar.jpg" height="220" width="1941"/></a></figure>
 !! end
 
+!! test
+Image: thumbnails of the default size use a square bounding box.
+!! options
+thumbsize=220
+!! wikitext
+[[File:Foobar.jpg|thumb|landscape thumb]]
+
+[[File:Foobar.jpg|frameless|landscape frameless]]
+
+[[File:Portrait.png|thumb|should use 220x220px bounding box]]
+
+[[File:Portrait.png|frameless|should use 220x220px bounding box]]
+
+[[File:Portrait.svg|thumb|should use 220x220px bounding box]]
+
+[[File:Portrait.svg|frameless|should use 220x220px bounding box]]
+!! html/php
+<div class="thumb tright"><div class="thumbinner" style="width:222px;"><a href="/wiki/File:Foobar.jpg" class="image"><img alt="" src="http://example.com/images/thumb/3/3a/Foobar.jpg/220px-Foobar.jpg" width="220" height="25" class="thumbimage" srcset="http://example.com/images/thumb/3/3a/Foobar.jpg/330px-Foobar.jpg 1.5x, http://example.com/images/thumb/3/3a/Foobar.jpg/440px-Foobar.jpg 2x" /></a>  <div class="thumbcaption"><div class="magnify"><a href="/wiki/File:Foobar.jpg" class="internal" title="Enlarge"><img src="/skins/common/images/magnify-clip.png" width="15" height="11" alt="" /></a></div>landscape thumb</div></div></div>
+<p><a href="/wiki/File:Foobar.jpg" class="image" title="landscape frameless"><img alt="landscape frameless" src="http://example.com/images/thumb/3/3a/Foobar.jpg/220px-Foobar.jpg" width="220" height="25" srcset="http://example.com/images/thumb/3/3a/Foobar.jpg/330px-Foobar.jpg 1.5x, http://example.com/images/thumb/3/3a/Foobar.jpg/440px-Foobar.jpg 2x" /></a>
+</p>
+<div class="thumb tright"><div class="thumbinner" style="width:167px;"><a href="/wiki/File:Portrait.png" class="image"><img alt="" src="http://example.com/images/thumb/9/93/Portrait.png/165px-Portrait.png" width="165" height="220" class="thumbimage" srcset="http://example.com/images/9/93/Portrait.png 1.5x, http://example.com/images/9/93/Portrait.png 2x" /></a>  <div class="thumbcaption"><div class="magnify"><a href="/wiki/File:Portrait.png" class="internal" title="Enlarge"><img src="/skins/common/images/magnify-clip.png" width="15" height="11" alt="" /></a></div>should use 220x220px bounding box</div></div></div>
+<p><a href="/wiki/File:Portrait.png" class="image" title="should use 220x220px bounding box"><img alt="should use 220x220px bounding box" src="http://example.com/images/thumb/9/93/Portrait.png/165px-Portrait.png" width="165" height="220" srcset="http://example.com/images/9/93/Portrait.png 1.5x, http://example.com/images/9/93/Portrait.png 2x" /></a>
+</p>
+<div class="thumb tright"><div class="thumbinner" style="width:167px;"><a href="/wiki/File:Portrait.svg" class="image"><img alt="" src="http://example.com/images/thumb/f/fc/Portrait.svg/165px-Portrait.svg.png" width="165" height="220" class="thumbimage" srcset="http://example.com/images/thumb/f/fc/Portrait.svg/247px-Portrait.svg.png 1.5x, http://example.com/images/thumb/f/fc/Portrait.svg/330px-Portrait.svg.png 2x" /></a>  <div class="thumbcaption"><div class="magnify"><a href="/wiki/File:Portrait.svg" class="internal" title="Enlarge"><img src="/skins/common/images/magnify-clip.png" width="15" height="11" alt="" /></a></div>should use 220x220px bounding box</div></div></div>
+<p><a href="/wiki/File:Portrait.svg" class="image" title="should use 220x220px bounding box"><img alt="should use 220x220px bounding box" src="http://example.com/images/thumb/f/fc/Portrait.svg/165px-Portrait.svg.png" width="165" height="220" srcset="http://example.com/images/thumb/f/fc/Portrait.svg/247px-Portrait.svg.png 1.5x, http://example.com/images/thumb/f/fc/Portrait.svg/330px-Portrait.svg.png 2x" /></a>
+</p>
+!! end
+
+!! test
+Image: bitmap thumbnails reduce only if thumb size is smaller than inherent size.
+!! options
+thumbsize=300
+!! wikitext
+[[File:Portrait.png|thumb|should use inherent 180x240px size]]
+
+[[File:Portrait.png|frameless|should use inherent 180x240px size]]
+
+[[File:Portrait.svg|thumb|will resize to 300x300px]]
+
+[[File:Portrait.svg|frameless|will resize to 300x300px]]
+!! html/php
+<div class="thumb tright"><div class="thumbinner" style="width:182px;"><a href="/wiki/File:Portrait.png" class="image"><img alt="" src="http://example.com/images/9/93/Portrait.png" width="180" height="240" class="thumbimage" /></a>  <div class="thumbcaption"><div class="magnify"><a href="/wiki/File:Portrait.png" class="internal" title="Enlarge"><img src="/skins/common/images/magnify-clip.png" width="15" height="11" alt="" /></a></div>should use inherent 180x240px size</div></div></div>
+<p><a href="/wiki/File:Portrait.png" class="image" title="should use inherent 180x240px size"><img alt="should use inherent 180x240px size" src="http://example.com/images/9/93/Portrait.png" width="180" height="240" /></a>
+</p>
+<div class="thumb tright"><div class="thumbinner" style="width:227px;"><a href="/wiki/File:Portrait.svg" class="image"><img alt="" src="http://example.com/images/thumb/f/fc/Portrait.svg/225px-Portrait.svg.png" width="225" height="300" class="thumbimage" srcset="http://example.com/images/thumb/f/fc/Portrait.svg/337px-Portrait.svg.png 1.5x, http://example.com/images/thumb/f/fc/Portrait.svg/450px-Portrait.svg.png 2x" /></a>  <div class="thumbcaption"><div class="magnify"><a href="/wiki/File:Portrait.svg" class="internal" title="Enlarge"><img src="/skins/common/images/magnify-clip.png" width="15" height="11" alt="" /></a></div>will resize to 300x300px</div></div></div>
+<p><a href="/wiki/File:Portrait.svg" class="image" title="will resize to 300x300px"><img alt="will resize to 300x300px" src="http://example.com/images/thumb/f/fc/Portrait.svg/225px-Portrait.svg.png" width="225" height="300" srcset="http://example.com/images/thumb/f/fc/Portrait.svg/337px-Portrait.svg.png 1.5x, http://example.com/images/thumb/f/fc/Portrait.svg/450px-Portrait.svg.png 2x" /></a>
+</p>
+!! end
+
 ###################
 
 !! test

diff --git a/tests/phpunit/data/media/Portrait.png b/tests/phpunit/data/media/Portrait.png
new file mode 100644 (file)
index 0000000..b5a92f6
Binary files /dev/null and b/tests/phpunit/data/media/Portrait.png differ

diff --git a/tests/phpunit/includes/parser/NewParserTest.php b/tests/phpunit/includes/parser/NewParserTest.php
index 14bcac0..26efa4a 100644 (file)
--- a/tests/phpunit/includes/parser/NewParserTest.php
+++ b/tests/phpunit/includes/parser/NewParserTest.php
@@ -285,6 +285,34 @@ class NewParserTest extends MediaWikiTestCase {
                                        'fileExists'  => true
                        ), $this->db->timestamp( '20010115123500' ), $user );
                }
+               $image = wfLocalFile( Title::makeTitle( NS_FILE, 'Portrait.png' ) );
+               if ( !$this->db->selectField( 'image', '1', array( 'img_name' => $image->getName() ) ) ) {
+                       $image->recordUpload2( '', 'Upload of tall bitmap', 'Some tall bitmap', array(
+                                       'size'        => 12345,
+                                       'width'       => 180,
+                                       'height'      => 240,
+                                       'bits'        => 8,
+                                       'media_type'  => MEDIATYPE_BITMAP,
+                                       'mime'        => 'image/png',
+                                       'metadata'    => serialize( array() ),
+                                       'sha1'        => wfBaseConvert( '', 16, 36, 31 ),
+                                       'fileExists'  => true
+                       ), $this->db->timestamp( '20140515134200' ), $user );
+               }
+               $image = wfLocalFile( Title::makeTitle( NS_FILE, 'Portrait.svg' ) );
+               if ( !$this->db->selectField( 'image', '1', array( 'img_name' => $image->getName() ) ) ) {
+                       $image->recordUpload2( '', 'Upload of tall SVG', 'Some tall SVG', array(
+                                       'size'        => 12345,
+                                       'width'       => 180,
+                                       'height'      => 240,
+                                       'bits'        => 24,
+                                       'media_type'  => MEDIATYPE_DRAWING,
+                                       'mime'        => 'image/svg+xml',
+                                       'metadata'    => serialize( array() ),
+                                       'sha1'        => wfBaseConvert( '', 16, 36, 31 ),
+                                       'fileExists'  => true
+                       ), $this->db->timestamp( '20140325124200' ), $user );
+               }
 
                # A DjVu file
                $image = wfLocalFile( Title::makeTitle( NS_FILE, 'LoremIpsum.djvu' ) );
@@ -499,6 +527,10 @@ class NewParserTest extends MediaWikiTestCase {
                $backend->store( array(
                        'src' => "$IP/skins/monobook/headbg.jpg", 'dst' => "$base/local-public/0/09/Bad.jpg"
                ) );
+               $backend->prepare( array( 'dir' => "$base/local-public/9/93" ) );
+               $backend->store( array(
+                       'src' => "$IP/tests/phpunit/data/media/Portrait.png", 'dst' => "$base/local-public/9/93/Portrait.png"
+               ) );
                $backend->prepare( array( 'dir' => "$base/local-public/5/5f" ) );
                $backend->store( array(
                        'src' => "$IP/tests/phpunit/data/media/LoremIpsum.djvu", 'dst' => "$base/local-public/5/5f/LoremIpsum.djvu"
@@ -513,6 +545,15 @@ class NewParserTest extends MediaWikiTestCase {
                $backend->quickCreate( array(
                        'content' => $data, 'dst' => "$base/local-public/f/ff/Foobar.svg"
                ) );
+
+               $data = '<?xml version="1.0" encoding="utf-8"?>' .
+                       '<svg xmlns="http://www.w3.org/2000/svg"' .
+                       ' version="1.1" width="180" height="240"/>';
+
+               $backend->prepare( array( 'dir' => "$base/local-public/f/fc" ) );
+               $backend->quickCreate( array(
+                       'content' => $data, 'dst' => "$base/local-public/f/fc/Portrait.svg"
+               ) );
        }
 
        /**
@@ -587,6 +628,9 @@ class NewParserTest extends MediaWikiTestCase {
                                "$base/local-thumb/5/5f/LoremIpsum.djvu/page2-3720px-LoremIpsum.djvu.jpg",
                                "$base/local-thumb/5/5f/LoremIpsum.djvu/page2-4960px-LoremIpsum.djvu.jpg",
 
+                               "$base/local-public/9/93/Portrait.png",
+                               "$base/local-thumb/9/93/Portrait.png/165px-Portrait.png",
+
                                "$base/local-public/f/ff/Foobar.svg",
                                "$base/local-thumb/f/ff/Foobar.svg/180px-Foobar.svg.png",
                                "$base/local-thumb/f/ff/Foobar.svg/2000px-Foobar.svg.png",
@@ -598,6 +642,14 @@ class NewParserTest extends MediaWikiTestCase {
                                "$base/local-thumb/f/ff/Foobar.svg/langde-270px-Foobar.svg.png",
                                "$base/local-thumb/f/ff/Foobar.svg/langde-360px-Foobar.svg.png",
 
+                               "$base/local-public/f/fc/Portrait.svg",
+                               "$base/local-thumb/f/fc/Portrait.svg/165px-Portrait.svg.png",
+                               "$base/local-thumb/f/fc/Portrait.svg/225px-Portrait.svg.png",
+                               "$base/local-thumb/f/fc/Portrait.svg/247px-Portrait.svg.png",
+                               "$base/local-thumb/f/fc/Portrait.svg/330px-Portrait.svg.png",
+                               "$base/local-thumb/f/fc/Portrait.svg/337px-Portrait.svg.png",
+                               "$base/local-thumb/f/fc/Portrait.svg/450px-Portrait.svg.png",
+
                                "$base/local-public/math/f/a/5/fa50b8b616463173474302ca3e63586b.png",
                        )
                );