r79036 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r79035‎ | r79036 | r79037 >
Date:00:03, 27 December 2010
Author:happy-melon
Status:ok (Comments)
Tags:
Comment:
(bug 268) add Special:PermanentLink/### as an internally-linkable redirect to index.php?oldid=###.

Also refactor SpecialPage::getRedirectQuery() to return an associative array to pass to wfArrayToCGI(), rather than bodging the query string together internally.
Modified paths:
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/SpecialPage.php
@@ -192,6 +192,7 @@
193193 'Mypage' => 'SpecialMypage',
194194 'Mytalk' => 'SpecialMytalk',
195195 'Myuploads' => 'SpecialMyuploads',
 196+ 'PermanentLink' => 'SpecialPermanentLink',
196197 'Revisiondelete' => 'SpecialRevisionDelete',
197198 'RevisionMove' => 'SpecialRevisionMove',
198199 'Specialpages' => 'SpecialSpecialpages',
@@ -541,12 +542,18 @@
542543 # Check for redirect
543544 if ( !$including ) {
544545 $redirect = $page->getRedirect( $par );
545 - if ( $redirect ) {
546 - $query = $page->getRedirectQuery();
 546+ $query = $page->getRedirectQuery();
 547+ if ( $redirect instanceof Title ) {
547548 $url = $redirect->getFullUrl( $query );
548549 $wgOut->redirect( $url );
549550 wfProfileOut( __METHOD__ );
550551 return $redirect;
 552+ } elseif( $redirect === true ) {
 553+ global $wgScript;
 554+ $url = $wgScript . '?' . wfArrayToCGI( $query );
 555+ $wgOut->redirect( $url );
 556+ wfProfileOut( __METHOD__ );
 557+ return $redirect;
551558 }
552559 }
553560
@@ -929,16 +936,20 @@
930937 function getRedirectQuery() {
931938 global $wgRequest;
932939 $params = array();
 940+
933941 foreach( $this->mAllowedRedirectParams as $arg ) {
934 - if( ( $val = $wgRequest->getVal( $arg, null ) ) !== null )
935 - $params[] = $arg . '=' . $val;
 942+ if( $wgRequest->getVal( $arg, null ) !== null ){
 943+ $params[$arg] = $wgRequest->getVal( $arg );
 944+ }
936945 }
937 -
 946+
938947 foreach( $this->mAddedRedirectParams as $arg => $val ) {
939 - $params[] = $arg . '=' . $val;
 948+ $params[$arg] = $val;
940949 }
941 -
942 - return count( $params ) ? implode( '&', $params ) : false;
 950+
 951+ return count( $params )
 952+ ? $params
 953+ : false;
943954 }
944955 }
945956
@@ -988,7 +999,8 @@
9891000 }
9901001 }
9911002
992 -/** SpecialMypage, SpecialMytalk and SpecialMycontributions special pages
 1003+/**
 1004+ * SpecialMypage, SpecialMytalk and SpecialMycontributions special pages
9931005 * are used to get user independant links pointing to the user page, talk
9941006 * page and list of contributions.
9951007 * This can let us cache a single copy of any generated content for all
@@ -1062,9 +1074,25 @@
10631075 parent::__construct( 'Myuploads' );
10641076 $this->mAllowedRedirectParams = array( 'limit' );
10651077 }
1066 -
 1078+
10671079 function getRedirect( $subpage ) {
10681080 global $wgUser;
10691081 return SpecialPage::getTitleFor( 'Listfiles', $wgUser->getName() );
10701082 }
10711083 }
 1084+
 1085+/**
 1086+ * Redirect to Special:Listfiles?user=$wgUser
 1087+ */
 1088+class SpecialPermanentLink extends UnlistedSpecialPage {
 1089+ function __construct() {
 1090+ parent::__construct( 'PermanentLink' );
 1091+ $this->mAllowedRedirectParams = array();
 1092+ }
 1093+
 1094+ function getRedirect( $subpage ) {
 1095+ $subpage = intval( $subpage );
 1096+ $this->mAddedRedirectParams['oldid'] = $subpage;
 1097+ return true;
 1098+ }
 1099+}
Index: trunk/phase3/languages/messages/MessagesEn.php
@@ -445,6 +445,7 @@
446446 'Mytalk' => array( 'MyTalk' ),
447447 'Mycontributions' => array( 'MyContributions' ),
448448 'Myuploads' => array( 'MyUploads' ),
 449+ 'PermanentLink' => array( 'PermanentLink' ),
449450 'Listadmins' => array( 'ListAdmins' ),
450451 'Listbots' => array( 'ListBots' ),
451452 'Popularpages' => array( 'PopularPages' ),

Sign-offs

UserFlagDate
Jarry1250tested21:26, 6 March 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r79037Follow-up r79036: update comment.happy-melon00:07, 27 December 2010
r79059Special:Permalink as alias for Special:PermanentLink (added in r79036)churchofemacs14:21, 27 December 2010
r86443Quick comments for r79036aaron23:18, 19 April 2011

Comments

#Comment by MZMcBride (talk | contribs)   00:11, 27 December 2010

This should have a "Special:Permalink" alias, I think.

+/**
+ * Redirect to Special:Listfiles?user=$wgUser
+ */

That looks wrong (never mind, fixed in r79037).

More fundamentally, permalinks are a subset of (the implicit) ?action=view. It's been long-discussed to have something like "Special:Action/view/page title" or some equivalent to kill off a lot of the uses of {{fullurl:}}. I'm not sure this is the best implementation forward given its general lack of expandability (a special page for every action is a bit weird), though I also don't have a problem making permalinks easier for people.

To that end, it might be nice if the "Permanent link" link in the sidebar had a pop-up or used this syntax. I guess that's outside the scope of this revision, though.

#Comment by Happy-melon (talk | contribs)   14:12, 27 December 2010

I'd say that a separate special page for each action actually makes more sense, as you don't need to have one syntax for every action. Most actions take one or more optional parameters, and which ones are a) permissible, b) useful-to-have-in-internal-link, and c) practical to pass, varies from action to action. For instance, we'd probably want Special:Edit/Foobar/20 to edit section 20 of Foobar, but Special:History/Foobar/20 to show 20 revisions of Foobar's history.

#Comment by MZMcBride (talk | contribs)   20:43, 27 December 2010

I think what you're saying is reasonable. Really I'm trying to get a more holistic (or at least expandable) approach in place rather than the usual piecemeal approach that MediaWiki development often takes. I don't see anything wrong with the approach you've taken here, but I do want to make sure we don't, going forward, have another mess like "Special:MovePage" v. every other action, for example. Inconsistency kills me.

#Comment by Church of emacs (talk | contribs)   14:25, 27 December 2010

Added Special:Permalink as alias in r79059 (I actually noted your comment after adding that alias).

Status & tagging log