From f8d0f0a7800606eea3d40b7091ae168ca2e34458 Mon Sep 17 00:00:00 2001 From: Brian Wolff Date: Tue, 16 Apr 2013 18:34:43 -0300 Subject: [PATCH] Fix getPageDimensions failure handling getPageDimensions returning false for failure wasn't being handled properly, causing ugly output. The doc comment on that method was wrong as well. Most notably causing random whitespace output: https://commons.wikimedia.org/wiki/?curid=22151015 (screenshot: http://i.imgur.com/c21EpVx.png). Bug: 41281 Change-Id: I1a49474309e15808928f877dfc29ae366d028928 --- RELEASE-NOTES-1.22 | 1 + includes/filerepo/file/LocalFile.php | 12 ++++++++---- includes/media/MediaHandler.php | 24 +++++++++++++++++------- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22 index 997dbc6c21..2c9b337379 100644 --- a/RELEASE-NOTES-1.22 +++ b/RELEASE-NOTES-1.22 @@ -153,6 +153,7 @@ production. warning will instead be issued if 'title' is non-default, unless no props are requested. * Special:Recentchangeslinked will now include upload log entries +* (bug 41281) Fixed ugly output if file size could not be extracted for multi-page media. === API changes in 1.22 === * (bug 25553) The JSON output formatter now leaves forward slashes unescaped diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index 12f24c5725..3be66d3ff3 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -604,7 +604,7 @@ class LocalFile extends File { * Return the width of the image * * @param $page int - * @return bool|int Returns false on error + * @return int */ public function getWidth( $page = 1 ) { $this->load(); @@ -614,7 +614,9 @@ class LocalFile extends File { if ( $dim ) { return $dim['width']; } else { - return false; + // For non-paged media, the false goes through an + // intval, turning failure into 0, so do same here. + return 0; } } else { return $this->width; @@ -625,7 +627,7 @@ class LocalFile extends File { * Return the height of the image * * @param $page int - * @return bool|int Returns false on error + * @return int */ public function getHeight( $page = 1 ) { $this->load(); @@ -635,7 +637,9 @@ class LocalFile extends File { if ( $dim ) { return $dim['height']; } else { - return false; + // For non-paged media, the false goes through an + // intval, turning failure into 0, so do same here. + return 0; } } else { return $this->height; diff --git a/includes/media/MediaHandler.php b/includes/media/MediaHandler.php index 3204fd7b49..2e8d41dee4 100644 --- a/includes/media/MediaHandler.php +++ b/includes/media/MediaHandler.php @@ -332,18 +332,28 @@ abstract class MediaHandler { * Get an associative array of page dimensions * Currently "width" and "height" are understood, but this might be * expanded in the future. - * Returns false if unknown or if the document is not multi-page. + * Returns false if unknown. + * + * It is expected that handlers for paged media (e.g. DjVuHandler) + * will override this method so that it gives the correct results + * for each specific page of the file, using the $page argument. + * + * @note For non-paged media, use getImageSize. * * @param $image File - * @param $page Unused, left for backcompatibility? - * @return array + * @param $page What page to get dimensions of + * @return array|bool */ function getPageDimensions( $image, $page ) { $gis = $this->getImageSize( $image, $image->getLocalRefPath() ); - return array( - 'width' => $gis[0], - 'height' => $gis[1] - ); + if ( $gis ) { + return array( + 'width' => $gis[0], + 'height' => $gis[1] + ); + } else { + return false; + } } /** -- 2.20.1