r66566 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r66565‎ | r66566 | r66567 >
Date:15:15, 17 May 2010
Author:hartman
Status:resolved (Comments)
Tags:
Comment:
Let old skins take into account $wgUploadNavigationUrl.
Also fixes old skins to take into account the actual uploadrights of users.
Fixes bug 23563
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)
  • /trunk/phase3/skins/CologneBlue.php (modified) (history)
  • /trunk/phase3/skins/Nostalgia.php (modified) (history)
  • /trunk/phase3/skins/Standard.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/CologneBlue.php
@@ -307,8 +307,8 @@
308308 . $this->specialLink( 'newpages' )
309309 . $sep . $this->specialLink( 'listfiles' )
310310 . $sep . $this->specialLink( 'statistics' );
311 - if ( $wgUser->isLoggedIn() && $wgEnableUploads ) {
312 - $s .= $sep . $this->specialLink( 'upload' );
 311+ if( UploadBase::isEnabled() && UploadBase::isAllowed( $wgUser ) === true ) {
 312+ $s .= $sep . $this->uploadLink();
313313 }
314314
315315 global $wgSiteSupportPage;
Index: trunk/phase3/skins/Standard.php
@@ -264,9 +264,10 @@
265265 $s .= "\n<br /><hr class='sep' />";
266266 }
267267
268 - if ( $wgUser->isLoggedIn() && ( $wgEnableUploads || $wgRemoteUploads ) ) {
269 - $s .= $this->specialLink( 'upload' ) . $sep;
 268+ if( UploadBase::isEnabled() && UploadBase::isAllowed( $wgUser ) === true ) {
 269+ $s .= $this->uploadLink() . $sep;
270270 }
 271+
271272 $s .= $this->specialLink( 'specialpages' );
272273
273274 global $wgSiteSupportPage;
Index: trunk/phase3/skins/Nostalgia.php
@@ -89,9 +89,10 @@
9090 /* show my preferences link */
9191 $s .= $sep . $this->specialLink( 'preferences' );
9292 /* show upload file link */
93 - if ( $wgEnableUploads ) {
94 - $s .= $sep . $this->specialLink( 'upload' );
 93+ if( UploadBase::isEnabled() && UploadBase::isAllowed( $wgUser ) === true ) {
 94+ $s .= $sep . $this->uploadLink();
9595 }
 96+
9697 /* show log out link */
9798 $s .= $sep . $this->specialLink( 'userlogout' );
9899 }
Index: trunk/phase3/includes/Skin.php
@@ -1988,6 +1988,25 @@
19891989 );
19901990 }
19911991
 1992+ function uploadLink() {
 1993+ global $wgUploadNavigationUrl;
 1994+
 1995+ if( $wgUploadNavigationUrl ) {
 1996+ $title = Title::newFromText( $wgUploadNavigationUrl );
 1997+ }
 1998+ if( !$title ) {
 1999+ $title = SpecialPage::getTitleFor('Upload');
 2000+ }
 2001+
 2002+ return $this->link(
 2003+ $title,
 2004+ wfMsgHtml( 'upload' ),
 2005+ array(),
 2006+ array(),
 2007+ array( 'known', 'noclasses' )
 2008+ );
 2009+ }
 2010+
19922011 /* these are used extensively in SkinTemplate, but also some other places */
19932012 static function makeMainPageUrl( $urlaction = '' ) {
19942013 $title = Title::newMainPage();
Index: trunk/phase3/RELEASE-NOTES
@@ -166,6 +166,7 @@
167167 * Special:Userrights didn't recognize user as changing his/her own rights if user did not capitalize first letter of username.
168168 * (bug 23507) Add styles for printing wikitables
169169 * (bug 19586) Avoid JS errors in mwsuggest when using old browsers such as Opera 8.
 170+* (bug 23563) Old skins now support $wgUploadNavigationUrl and take into account upload rights.
170171
171172 === API changes in 1.17 ===
172173 * (bug 22738) Allow filtering by action type on query=logevent

Follow-up revisions

RevisionCommit summaryAuthorDate
r66589Follow up r66566. Fixes uninited value checkhartman21:45, 17 May 2010
r66606Followup r66566. wgUploadNavigationUrl is an external link.hartman11:41, 18 May 2010
r68341Follow up r66566. Rename uploadLink() to getUploadLink() per Bryan's commentshartman22:24, 20 June 2010
r74627Remove $wgRemoteUploads. It was not well supported and superseded by $wgUploa...hartman15:20, 11 October 2010

Comments

#Comment by Bryan (talk | contribs)   16:48, 17 May 2010
+	function uploadLink() {

Function names should begin with a verb that indicates what is does. Since this function does not upload a link, it should be called getUploadLink instead.


#Comment by TheDJ (talk | contribs)   20:32, 17 May 2010

Well since all the 15 or so other "getlink" type functions of Skin.php were not adhering to this standard, I thought it was best to keep it consistent. But I have no problem in changing it.

#Comment by Siebrand (talk | contribs)   21:14, 17 May 2010

PHP Notice: Undefined variable: title in /www/w/includes/Skin.php on line 1997

Please init $title.

#Comment by TheDJ (talk | contribs)   21:48, 17 May 2010

$wgUploadNavigationUrl can also be an external link, and link() doesn't support external links....

#Comment by TheDJ (talk | contribs)   23:35, 17 May 2010

OK, i'm not sure what to do about this, doesn't seem like we have much standard methods for cases like this. I was thinking the following. If anyone has a better idea, please let me know.

                if( $wgUploadNavigationUrl ) {
                        return $this->makeExternalLink( $wgUploadNavigationUrl, wfMsgHtml( 'upload' ), false, null, array( 'class' => '') );
                } else {
                        $title = SpecialPage::getTitleFor('Upload');

                        return $this->link(
                                $title,
                                wfMsgHtml( 'upload' ),
                                array(),
                                array(),
                                array( 'known', 'noclasses' )
                        );
                }
#Comment by Simetrical (talk | contribs)   23:38, 17 May 2010

Maybe you could extend link() to support external links. It currently only accepts Titles, so you could overload it by using strings for external links, I'd think.

#Comment by TheDJ (talk | contribs)   11:43, 18 May 2010

Should be solved with r66606

#Comment by Nikerabbit (talk | contribs)   13:41, 8 August 2010

This renders $wgRemoteUploads unused. Should it be removed?

#Comment by Bryan (talk | contribs)   18:52, 9 October 2010

Yes I think so.

#Comment by TheDJ (talk | contribs)   15:15, 11 October 2010

It was only used in the Standard skin ? Weird, wasn't a very reliable/useful option then for the last years! I'll remove it from Defaults, and will update the mediawiki documentation.

#Comment by TheDJ (talk | contribs)   15:22, 11 October 2010

Fixed, and I scored the 10000'th comment ! :D Yeah Code Review comments.

Status & tagging log