r92598 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r92597‎ | r92598 | r92599 >
Date:23:09, 19 July 2011
Author:robin
Status:ok (Comments)
Tags:
Comment:
$wgUploadNavigationUrl should be used for file redlinks if $wgUploadMissingFileUrl is not set. The first was used for this until the second was introduced in r69997 (1.17). According to the comments there it was broken in 1.16 but as far as I tested it worked in 1.16. In any case, the old behavior should be restored as most WMF wikis have set wgUploadNavigationUrl but not wgUploadMissingFileUrl.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/Linker.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
@@ -17,6 +17,9 @@
1818 cache is used.
1919
2020 === Bug fixes in 1.19 ===
 21+* $wgUploadNavigationUrl should be used for file redlinks if
 22+ $wgUploadMissingFileUrl is not set. The first was used for this
 23+ until the second was introduced in 1.17.
2124
2225 === API changes in 1.19 ===
2326 * (bug 19838) siprop=interwikimap can now use the interwiki cache.
Index: trunk/phase3/includes/Linker.php
@@ -768,7 +768,7 @@
769769 * @return String
770770 */
771771 public static function makeBrokenImageLinkObj( $title, $text = '', $query = '', $trail = '', $prefix = '', $time = false ) {
772 - global $wgEnableUploads, $wgUploadMissingFileUrl;
 772+ global $wgEnableUploads, $wgUploadMissingFileUrl, $wgUploadNavigationUrl;
773773 if ( ! $title instanceof Title ) {
774774 return "<!-- ERROR -->{$prefix}{$text}{$trail}";
775775 }
@@ -779,7 +779,7 @@
780780 if ( $text == '' )
781781 $text = htmlspecialchars( $title->getPrefixedText() );
782782
783 - if ( ( $wgUploadMissingFileUrl || $wgEnableUploads ) && !$currentExists ) {
 783+ if ( ( $wgUploadMissingFileUrl || $wgUploadNavigationUrl || $wgEnableUploads ) && !$currentExists ) {
784784 $redir = RepoGroup::singleton()->getLocalRepo()->checkRedirect( $title );
785785
786786 if ( $redir ) {
@@ -807,13 +807,15 @@
808808 * @return String: urlencoded URL
809809 */
810810 protected static function getUploadUrl( $destFile, $query = '' ) {
811 - global $wgUploadMissingFileUrl;
 811+ global $wgUploadMissingFileUrl, $wgUploadNavigationUrl;
812812 $q = 'wpDestFile=' . $destFile->getPartialUrl();
813813 if ( $query != '' )
814814 $q .= '&' . $query;
815815
816816 if ( $wgUploadMissingFileUrl ) {
817817 return wfAppendQuery( $wgUploadMissingFileUrl, $q );
 818+ } elseif( $wgUploadNavigationUrl ) {
 819+ return wfAppendQuery( $wgUploadNavigationUrl, $q );
818820 } else {
819821 $upload = SpecialPage::getTitleFor( 'Upload' );
820822 return $upload->getLocalUrl( $q );

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r69997Fixes for (bug 18885), r60593, r60979: The upload link for missing files can ...btongminh10:31, 27 July 2010

Comments

#Comment by TheDJ (talk | contribs)   05:48, 20 July 2011

It was broken in 1.16. The redlinks for thumbnails with missing files on en.wp would take you to some sort of incorrect location.

See also bug 18885 and r60593 (the latter is what caused the broken functionality I think). It think that is required here is a mass change to ALSO configure missinglinkurl for the wiki's that have uploadnavigationurl.

Bryan should know more.

#Comment by Bryan (talk | contribs)   07:37, 20 July 2011

DJ's summary is correct. I'm personally not really sure whether or not we should introduce this fallback though.

#Comment by SPQRobin (talk | contribs)   12:15, 20 July 2011

In my opinion it is superfluous to have to configure all wikis with UploadNavigationUrl also with UploadMissingFileUrl. It's better if it falls back, which was the old behavior, and which is more logical. Also, the docs @ mediawiki.org said it falls back, which was wrong and confusing for other people (I updated it).

Could you explain what was broken in 1.16? I don't see it.

(And why is this revision marked fixme?)

#Comment by SPQRobin (talk | contribs)   12:23, 20 July 2011

Ah I see what's the problem. The added wpDestFile is useless when UploadNavigationUrl is set to a normal page like Wikipedia:Upload.

#Comment by SPQRobin (talk | contribs)   12:40, 20 July 2011

So we could either

  • revert this commit and configure all wikis with UploadNavigationUrl also with UploadMissingFileUrl
  • leave this commit and configure wikis where UploadNavigationUrl goes to a non-special page (about half of the configured wikis), with UploadMissingFileUrl

In any case this should have been done back then (or at least the wikis should've been informed).

#Comment by Aaron Schulz (talk | contribs)   20:26, 9 September 2011

The later I suppose.

Status & tagging log