jenkins-bot has submitted this change and it was merged.

Change subject: Fix getPageDimensions failure handling
......................................................................


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
---
M RELEASE-NOTES-1.22
M includes/filerepo/file/LocalFile.php
M includes/media/MediaHandler.php
3 files changed, 26 insertions(+), 11 deletions(-)

Approvals:
  Aaron Schulz: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 997dbc6..2c9b337 100644
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -153,6 +153,7 @@
   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 12f24c5..3be66d3 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -604,7 +604,7 @@
         * 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 @@
                        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 @@
         * 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 @@
                        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 3204fd7..2e8d41d 100644
--- a/includes/media/MediaHandler.php
+++ b/includes/media/MediaHandler.php
@@ -332,18 +332,28 @@
         * 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;
+               }
        }
 
        /**

-- 
To view, visit https://gerrit.wikimedia.org/r/59530
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I1a49474309e15808928f877dfc29ae366d028928
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Brian Wolff <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Brian Wolff <[email protected]>
Gerrit-Reviewer: Matmarex <[email protected]>
Gerrit-Reviewer: TheDJ <[email protected]>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to