From 3c286bde96b1f8991096048d466f0123232f49a6 Mon Sep 17 00:00:00 2001 From: Brian Wolff Date: Sun, 19 Aug 2012 15:18:50 -0300 Subject: [PATCH] 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 --- RELEASE-NOTES-1.20 | 2 ++ includes/media/SVG.php | 14 ++++++++++--- includes/media/SVGMetadataExtractor.php | 10 ++++++++++ .../media/SVGMetadataExtractorTest.php | 20 ++++++++++++++----- 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/RELEASE-NOTES-1.20 b/RELEASE-NOTES-1.20 index c9bf279df6..a8c2b94b62 100644 --- 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 a5ce1fa6cd..a9d1758b32 100644 --- 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 89e1e0c14b..851fe428a6 100644 --- 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 526beae81b..3017dbbd96 100644 --- 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', ) ), ); -- 2.20.1