Follow-up to r44216/r44217 "Followup to r44204. Hardcoding image because it should...

It was noted that the existing code could fail for unusual article path settings including a suffix if only the 'articleUrl' was available. Pretty corner case, and probably breaks a lot of other things, but who knows. :)
Went ahead and refactored this a bit to handle the case and feel a little cleaner to me...
Dropped the private getDescBaseUrl() method since this seems a little easier to handle directly in getDescriptionUrl() now.

diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php
index 32f04ed..fdd9757 100644 (file)
--- a/includes/filerepo/FileRepo.php
+++ b/includes/filerepo/FileRepo.php
@@ -254,25 +254,6 @@ abstract class FileRepo {
                return $this->name;
        }
 
-       /**
-        * Get the file description page base URL, or false if there isn't one.
-        * @private
-        */
-       function getDescBaseUrl() {
-               if ( is_null( $this->descBaseUrl ) ) {
-                       if ( !is_null( $this->articleUrl ) ) {
-                               $this->descBaseUrl = str_replace( '$1',
-                                       wfUrlencode( 'Image' ) . ':', $this->articleUrl );
-                       } elseif ( !is_null( $this->scriptDirUrl ) ) {
-                               $this->descBaseUrl = $this->scriptDirUrl . '/index.php?title=' .
-                                       wfUrlencode( 'Image' ) . ':';
-                       } else {
-                               $this->descBaseUrl = false;
-                       }
-               }
-               return $this->descBaseUrl;
-       }
-
        /**
         * Get the URL of an image description page. May return false if it is
         * unknown or not applicable. In general this should only be called by the
@@ -283,12 +264,29 @@ abstract class FileRepo {
         * constructor, whereas local repositories use the local Title functions.
         */
        function getDescriptionUrl( $name ) {
-               $base = $this->getDescBaseUrl();
-               if ( $base ) {
-                       return $base . wfUrlencode( $name );
-               } else {
-                       return false;
+               $encName = wfUrlencode( $name );
+               if ( !is_null( $this->descBaseUrl ) ) {
+                       # "http://example.com/wiki/Image:"
+                       return $this->descBaseUrl . $encName;
+               }
+               if ( !is_null( $this->articleUrl ) ) {
+                       # "http://example.com/wiki/$1"
+                       #
+                       # We use "Image:" as the canonical namespace for
+                       # compatibility across all MediaWiki versions.
+                       return str_replace( '$1',
+                               "Image:$encName", $this->articleUrl );
                }
+               if ( !is_null( $this->scriptDirUrl ) ) {
+                       # "http://example.com/w"
+                       #
+                       # We use "Image:" as the canonical namespace for
+                       # compatibility across all MediaWiki versions,
+                       # and just sort of hope index.php is right. ;)
+                       return $this->scriptDirUrl .
+                               "/index.php?title=Image:$encName";
+               }
+               return false;
        }
 
        /**
@@ -303,9 +301,9 @@ abstract class FileRepo {
                                wfUrlencode( 'Image:' . $name ) .
                                '&action=render';
                } else {
-                       $descBase = $this->getDescBaseUrl();
-                       if ( $descBase ) {
-                               return wfAppendQuery( $descBase . wfUrlencode( $name ), 'action=render' );
+                       $descUrl = $this->getDescriptionUrl( $name );
+                       if ( $descUrl ) {
+                               return wfAppendQuery( $descUrl, 'action=render' );
                        } else {
                                return false;
                        }