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

Change subject: Do not require iiurlwidth when getting thumbnails.
......................................................................


Do not require iiurlwidth when getting thumbnails.

Some file types might not have a specific width/height. As an
example, TimedMediaHandler can specify thumbnails that are
"mid" sized instead of specific width. The requirement for
iiurlwidth was breaking some edge cases with ForeignAPIRepo
and TimedMediaHandler (Specificly with ?embedplayer=yes).

Do not require a width, but also verify that thumbnail parameters
are ok by running through normaliseParams (Formats that require
a width make this function return false if none is specified).

Change-Id: I8bb4c26db56c814ddfbd3e53ca98d05378c2a0bd
---
M RELEASE-NOTES-1.24
M includes/api/ApiQueryImageInfo.php
2 files changed, 43 insertions(+), 7 deletions(-)

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



diff --git a/RELEASE-NOTES-1.24 b/RELEASE-NOTES-1.24
index 79b8a49..67b47bf 100644
--- a/RELEASE-NOTES-1.24
+++ b/RELEASE-NOTES-1.24
@@ -262,6 +262,7 @@
   deprecated in favor of cmstarthexsortkey and cmendhexsortkey.
 * (bug 63326) Add blockedtimestamp field to output of blockinfo property for
   the list=allusers and list=users modules.
+* prop=imageinfo no longer requires iiurlwidth to be set when using iiurlparam.
 
 === Action API internal changes in 1.24 ===
 * Methods for handling continuation are added to ApiResult, so actions other
diff --git a/includes/api/ApiQueryImageInfo.php 
b/includes/api/ApiQueryImageInfo.php
index 5cc1454..b8f0890 100644
--- a/includes/api/ApiQueryImageInfo.php
+++ b/includes/api/ApiQueryImageInfo.php
@@ -237,9 +237,11 @@
                        $scale = array();
                        $scale['height'] = $params['urlheight'];
                } else {
-                       $scale = null;
                        if ( $params['urlparam'] ) {
-                               $this->dieUsage( "{$p}urlparam requires 
{$p}urlwidth", "urlparam_no_width" );
+                               // Audio files might not have a width/height.
+                               $scale = array();
+                       } else {
+                               $scale = null;
                        }
                }
 
@@ -256,6 +258,10 @@
         * @return array Array of parameters for transform.
         */
        protected function mergeThumbParams( $image, $thumbParams, $otherParams 
) {
+               if ( $thumbParams === null ) {
+                       // No scaling requested
+                       return null;
+               }
                if ( !isset( $thumbParams['width'] ) && isset( 
$thumbParams['height'] ) ) {
                        // We want to limit only by height in this situation, 
so pass the
                        // image's full width as the limiting width. But some 
file types
@@ -269,6 +275,7 @@
                }
 
                if ( !$otherParams ) {
+                       $this->checkParameterNormalise( $image, $thumbParams );
                        return $thumbParams;
                }
                $p = $this->getModulePrefix();
@@ -289,11 +296,11 @@
                        // handlers.
                        $this->setWarning( "Could not parse {$p}urlparam for " 
. $image->getName()
                                . '. Using only width and height' );
-
+                       $this->checkParameterNormalise( $image, $thumbParams );
                        return $thumbParams;
                }
 
-               if ( isset( $paramList['width'] ) ) {
+               if ( isset( $paramList['width'] ) && isset( 
$thumbParams['width'] ) ) {
                        if ( intval( $paramList['width'] ) != intval( 
$thumbParams['width'] ) ) {
                                $this->setWarning( "Ignoring width value set in 
{$p}urlparam ({$paramList['width']}) "
                                        . "in favor of width value derived from 
{$p}urlwidth/{$p}urlheight "
@@ -307,7 +314,33 @@
                        }
                }
 
-               return $thumbParams + $paramList;
+               $finalParams = $thumbParams + $paramList;
+               $this->checkParameterNormalise( $image, $finalParams );
+               return $finalParams;
+       }
+
+       /**
+        * Verify that the final image parameters can be normalised.
+        *
+        * This doesn't use the normalised parameters, since $file->transform
+        * expects the pre-normalised parameters, but doing the normalisation
+        * allows us to catch certain error conditions early (such as missing
+        * required parameter).
+        *
+        * @param $image File
+        * @param $finalParams array List of parameters to transform image with
+        */
+       protected function checkParameterNormalise( $image, $finalParams ) {
+               $h = $image->getHandler();
+               if ( !$h ) {
+                       return;
+               }
+               // Note: normaliseParams modifies the array in place, but we 
aren't interested
+               // in the actual normalised version, only if we can actually 
normalise them,
+               // so we use the functions scope to throw away the 
normalisations.
+               if ( !$h->normaliseParams( $image, $finalParams ) ) {
+                       $this->dieUsage( "Could not normalise image parameters 
for " . $image->getName(), "urlparamnormal" );
+               }
        }
 
        /**
@@ -716,8 +749,10 @@
                                        'no more than ' . self::TRANSFORM_LIMIT 
. ' scaled images will be returned.'
                        ),
                        'urlheight' => "Similar to {$p}urlwidth.",
-                       'urlparam' => array( "A handler specific parameter 
string. For example, pdf's ",
-                               "might use 'page15-100px'. {$p}urlwidth must be 
used and be consistent with {$p}urlparam" ),
+                       'urlparam' => array(
+                               "A handler specific parameter string. For 
example, pdf's ",
+                               "might use 'page15-100px'."
+                       ),
                        'limit' => 'How many image revisions to return per 
image',
                        'start' => 'Timestamp to start listing from',
                        'end' => 'Timestamp to stop listing at',

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8bb4c26db56c814ddfbd3e53ca98d05378c2a0bd
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Brian Wolff <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Brian Wolff <[email protected]>
Gerrit-Reviewer: Gilles <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

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

Reply via email to