Make the width/height in the SVG metadata box be in the SVG's units

This was suggested by a user at commons. The width/height in px is really
a MediaWiki thingy needed for rendering the image, from a metadata prespective
its the original units (aka the image should be 10cm, etc) that is
the information associated with the file.

The dimensions in pixels is still specified in the long description
of the image (the subtitle directly under the image).

Note: After gerrit change Ic58efbf2 is merged, this will cause no width or height
to be displayed for svg images with cached metadata, until they
get purged. I think that's perfectly ok as the width is
available elsewhere on the page.

p.s. I added one semi-unrelated code comment in SVGMetadataExtractor.

patchset 2,3: Fix test (hopefully, how come getting an up to date
version of PHPUnit is so difficult)
patchset 4: rebase
patchset 5: fix my accidental remove of a comment

Change-Id: I312fbd1c935a0095644c3b63c4929632c6f6e387

diff --git a/RELEASE-NOTES-1.20 b/RELEASE-NOTES-1.20
index c9bf279..a8c2b94 100644 (file)
--- a/RELEASE-NOTES-1.20
+++ b/RELEASE-NOTES-1.20
@@ -131,6 +131,8 @@ upgrade PHP if you have not done so prior to upgrading MediaWiki.
 * (bug 23226) Add |class= parameter to image links in order to add class(es) to HTML img tag.
 * (bug 39431) SVG animated status is now shown in long description
 * (bug 39376) jquery.form upgraded to 3.14
+* SVG files will now show the actual width in the SVG's specified units
+  in the metadata box.
 
 === Bug fixes in 1.20 ===
 * (bug 30245) Use the correct way to construct a log page title.

diff --git a/includes/media/SVG.php b/includes/media/SVG.php
index a5ce1fa..a9d1758 100644 (file)
--- a/includes/media/SVG.php
+++ b/includes/media/SVG.php
@@ -275,7 +275,15 @@ class SvgHandler extends ImageHandler {
        }
 
        function isMetadataValid( $image, $metadata ) {
-               return $this->unpackMetadata( $metadata ) !== false;
+               $meta = $this->unpackMetadata( $metadata );
+               if ( $meta === false ) {
+                       return self::METADATA_BAD;
+               }
+               if ( !isset( $meta['originalWidth'] ) ) {
+                       // Old but compatible
+                       return self::METADATA_COMPATIBLE;
+               }
+               return self::METADATA_GOOD;
        }
 
        function visibleMetadataFields() {
@@ -312,8 +320,8 @@ class SvgHandler extends ImageHandler {
                // Rename fields to be compatible with exif, so that
                // the labels for these fields work and reuse existing messages.
                $conversion = array(
-                       'width' => 'imagewidth',
-                       'height' => 'imagelength',
+                       'originalwidth' => 'imagewidth',
+                       'originalheight' => 'imagelength',
                        'description' => 'imagedescription',
                        'title' => 'objectname',
                );

diff --git a/includes/media/SVGMetadataExtractor.php b/includes/media/SVGMetadataExtractor.php
index 89e1e0c..851fe42 100644 (file)
--- a/includes/media/SVGMetadataExtractor.php
+++ b/includes/media/SVGMetadataExtractor.php
@@ -83,6 +83,12 @@ class SVGReader {
                $this->metadata['width'] = self::DEFAULT_WIDTH;
                $this->metadata['height'] = self::DEFAULT_HEIGHT;
 
+               // The size in the units specified by the SVG file
+               // (for the metadata box)
+               // Per the SVG spec, if unspecified, default to '100%'
+               $this->metadata['originalWidth'] = '100%';
+               $this->metadata['originalHeight'] = '100%';
+
                // Because we cut off the end of the svg making an invalid one. Complicated
                // try catch thing to make sure warnings get restored. Seems like there should
                // be a better way.
@@ -90,6 +96,8 @@ class SVGReader {
                try {
                        $this->read();
                } catch( Exception $e ) {
+                       // Note, if this happens, the width/height will be taken to be 0x0.
+                       // Should we consider it the default 512x512 instead?
                        wfRestoreWarnings();
                        throw $e;
                }
@@ -288,9 +296,11 @@ class SVGReader {
                }
                if( $this->reader->getAttribute('width') ) {
                        $width = $this->scaleSVGUnit( $this->reader->getAttribute('width'), $defaultWidth );
+                       $this->metadata['originalWidth'] = $this->reader->getAttribute( 'width' );
                }
                if( $this->reader->getAttribute('height') ) {
                        $height = $this->scaleSVGUnit( $this->reader->getAttribute('height'), $defaultHeight );
+                       $this->metadata['originalHeight'] = $this->reader->getAttribute( 'height' );
                }
 
                if( !isset( $width ) && !isset( $height ) ) {

diff --git a/tests/phpunit/includes/media/SVGMetadataExtractorTest.php b/tests/phpunit/includes/media/SVGMetadataExtractorTest.php
index 526beae..3017dbb 100644 (file)
--- a/tests/phpunit/includes/media/SVGMetadataExtractorTest.php
+++ b/tests/phpunit/includes/media/SVGMetadataExtractorTest.php
@@ -45,21 +45,27 @@ class SVGMetadataExtractorTest extends MediaWikiTestCase {
                                "$base/Wikimedia-logo.svg",
                                array(
                                        'width' => 1024,
-                                       'height' => 1024
+                                       'height' => 1024,
+                                       'originalWidth' => '1024',
+                                       'originalHeight' => '1024',
                                )
                        ),
                        array(
                                "$base/QA_icon.svg",
                                array(
                                        'width' => 60,
-                                       'height' => 60
+                                       'height' => 60,
+                                       'originalWidth' => '60',
+                                       'originalHeight' => '60',
                                )
                        ),
                        array(
                                "$base/Gtk-media-play-ltr.svg",
                                array(
                                        'width' => 60,
-                                       'height' => 60
+                                       'height' => 60,
+                                       'originalWidth' => '60.0000000',
+                                       'originalHeight' => '60.0000000',
                                )
                        ),
                        array(
@@ -67,7 +73,9 @@ class SVGMetadataExtractorTest extends MediaWikiTestCase {
                                // This file triggered bug 31719, needs entity expansion in the xmlns checks
                                array(
                                        'width' => 385,
-                                       'height' => 385
+                                       'height' => 385,
+                                       'originalWidth' => '385',
+                                       'originalHeight' => '385.0004883',
                                )
                        )
                );
@@ -89,7 +97,9 @@ class SVGMetadataExtractorTest extends MediaWikiTestCase {
                                array(
                                        'height' => 593,
                                        'metadata' => $metadata,
-                                       'width' => 959
+                                       'width' => 959,
+                                       'originalWidth' => '958.69',
+                                       'originalHeight' => '592.78998',
                                )
                        ),
                );