r51559 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r51558‎ \| r51559 \| r51560 >
Date:	22:42, 6 June 2009
Author:	siebrand
Status:	ok (Comments)
Tags:
Comment:	* replace some use of deprecated makeKnownLinkObj() by link() in core * use array type parameter instead of string to escapeLocalUrl(), getFullURL() and getFullUrl() for readability
Modified paths:	/trunk/phase3/includes/Article.php (modified) (history) /trunk/phase3/includes/CategoryPage.php (modified) (history) /trunk/phase3/includes/ChangesList.php (modified) (history) /trunk/phase3/includes/EditPage.php (modified) (history) /trunk/phase3/includes/FileDeleteForm.php (modified) (history) /trunk/phase3/includes/FileRevertForm.php (modified) (history) /trunk/phase3/includes/ImagePage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/FileRevertForm.php
—	—	@@ -114,7 +114,16 @@
115	115	global $wgOut, $wgUser;
116	116	$wgOut->setPageTitle( wfMsg( 'filerevert', $this->title->getText() ) );
117	117	$wgOut->setRobotPolicy( 'noindex,nofollow' );
118		~~- $wgOut->setSubtitle( wfMsg( 'filerevert-backlink', $wgUser->getSkin()->makeKnownLinkObj( $this->title ) ) );~~
	118	+ $wgOut->setSubtitle( wfMsg(
	119	+ 'filerevert-backlink',
	120	+ $wgUser->getSkin()->link(
	121	+ $this->title,
	122	+ null,
	123	+ array(),
	124	+ array(),
	125	+ array( 'known', 'noclasses' )
	126	+ )
	127	+ ) );
119	128	}
120	129
121	130	/**
Index: trunk/phase3/includes/CategoryPage.php
—	—	@@ -144,8 +144,13 @@
145	145	function addSubcategory( $title, $sortkey, $pageLength ) {
146	146	global $wgContLang;
147	147	// Subcategory; strip the 'Category' namespace from the link text.
148		~~- $this->children[] = $this->getSkin()->makeKnownLinkObj(~~
149		~~- $title, $wgContLang->convertHtml( $title->getText() ) );~~
	148	+ $this->children[] = $this->getSkin()->link(
	149	+ $title,
	150	+ $wgContLang->convertHtml( $title->getText() ).
	151	+ array(),
	152	+ array(),
	153	+ array( 'known', 'noclasses' )
	154	+ );
150	155
151	156	$this->children_start_char[] = $this->getSubcategorySortChar( $title, $sortkey );
152	157	}
—	—	@@ -191,7 +196,14 @@
192	197	global $wgContLang;
193	198	$titletext = $wgContLang->convertHtml( $title->getPrefixedText() );
194	199	$this->articles[] = $isRedirect
195		~~- ? '<span class="redirect-in-category">' . $this->getSkin()->makeKnownLinkObj( $title, $titletext ) . '</span>'~~
	200	+ ? '<span class="redirect-in-category">' .
	201	+ $this->getSkin()->link(
	202	+ $title,
	203	+ $titletext,
	204	+ array(),
	205	+ array(),
	206	+ array( 'known', 'noclasses' )
	207	+ ) . '</span>'
196	208	: $this->getSkin()->makeSizeLinkObj( $pageLength, $title, $titletext );
197	209	$this->articles_start_char[] = $wgContLang->convert( $wgContLang->firstChar( $sortkey ) );
198	210	}
Index: trunk/phase3/includes/Article.php
—	—	@@ -836,7 +836,13 @@
837	837	// This is an internally redirected page view.
838	838	// We'll need a backlink to the source page for navigation.
839	839	if( wfRunHooks( 'ArticleViewRedirect', array( &$this ) ) ) {
840		~~- $redir = $sk->makeKnownLinkObj( $this->mRedirectedFrom, '', 'redirect=no' );~~
	840	+ $redir = $sk->link(
	841	+ $this->mRedirectedFrom,
	842	+ null,
	843	+ array(),
	844	+ array( 'redirect' => 'no' ),
	845	+ array( 'known', 'noclasses' )
	846	+ );
841	847	$s = wfMsgExt( 'redirectedfrom', array( 'parseinline', 'replaceafter' ), $redir );
842	848	$wgOut->setSubtitle( $s );
843	849
—	—	@@ -1040,15 +1046,24 @@
1041	1047
1042	1048	# If we have been passed an &rcid= parameter, we want to give the user a
1043	1049	# chance to mark this new article as patrolled.
1044		~~- if( !empty($rcid) && $this->mTitle->exists() && $this->mTitle->quickUserCan('patrol') ) {~~
	1050	+ if( !empty( $rcid ) && $this->mTitle->exists() && $this->mTitle->quickUserCan( 'patrol' ) ) {
1045	1051	$wgOut->addHTML(
1046	1052	"<div class='patrollink'>" .
1047		~~- wfMsgHtml( 'markaspatrolledlink',~~
1048		~~- $sk->makeKnownLinkObj( $this->mTitle, wfMsgHtml('markaspatrolledtext'),~~
1049		~~- "action=markpatrolled&rcid=$rcid" )~~
1050		~~- ) .~~
	1053	+ wfMsgHtml(
	1054	+ 'markaspatrolledlink',
	1055	+ $sk->link(
	1056	+ $this->mTitle,
	1057	+ wfMsgHtml( 'markaspatrolledtext' ),
	1058	+ array(),
	1059	+ array(
	1060	+ 'action' => 'markpatrolled',
	1061	+ 'rcid' => $rcid
	1062	+ ),
	1063	+ array( 'known', 'noclasses' )
	1064	+ )
	1065	+ ) .
1051	1066	'</div>'
1052		~~- );~~
	1067	+ );
1053	1068	}
1054	1069
1055	1070	# Trackbacks
—	—	@@ -1123,7 +1138,13 @@
1124	1139	// the loop prepends the arrow image before the link, so the first case needs to be outside
1125	1140	$title = array_shift( $target );
1126	1141	if( $forceKnown ) {
1127		~~- $link = $sk->makeKnownLinkObj( $title, htmlspecialchars( $title->getFullText() ) );~~
	1142	+ $link = $sk->link(
	1143	+ $title,
	1144	+ htmlspecialchars( $title->getFullText() ),
	1145	+ array(),
	1146	+ array(),
	1147	+ array( 'known', 'noclasses' )
	1148	+ );
1128	1149	} else {
1129	1150	$link = $sk->link( $title, htmlspecialchars( $title->getFullText() ) );
1130	1151	}
—	—	@@ -1131,7 +1152,13 @@
1132	1153	foreach( $target as $rt ) {
1133	1154	if( $forceKnown ) {
1134	1155	$link .= '<img src="'.$imageUrl2.'" alt="'.$alt2.' " />'
1135		~~- . $sk->makeKnownLinkObj( $rt, htmlspecialchars( $rt->getFullText() ) );~~
	1156	+ . $sk->link(
	1157	+ $rt,
	1158	+ htmlspecialchars( $rt->getFullText() ),
	1159	+ array(),
	1160	+ array(),
	1161	+ array( 'known', 'noclasses' )
	1162	+ );
1136	1163	} else {
1137	1164	$link .= '<img src="'.$imageUrl2.'" alt="'.$alt2.' " />'
1138	1165	. $sk->link( $rt, htmlspecialchars( $rt->getFullText() ) );
—	—	@@ -2404,7 +2431,14 @@
2405	2432
2406	2433	wfDebug( "Article::confirmDelete\n" );
2407	2434
2408		~~- $wgOut->setSubtitle( wfMsgHtml( 'delete-backlink', $wgUser->getSkin()->makeKnownLinkObj( $this->mTitle ) ) );~~
	2435	+ $deleteBackLink = $wgUser->getSkin()->link(
	2436	+ $this->mTitle,
	2437	+ null,
	2438	+ array(),
	2439	+ array(),
	2440	+ array( 'known', 'noclasses' )
	2441	+ );
	2442	+ $wgOut->setSubtitle( wfMsgHtml( 'delete-backlink', $deleteBackLink ) );
2409	2443	$wgOut->setRobotPolicy( 'noindex,nofollow' );
2410	2444	$wgOut->addWikiMsg( 'confirmdeletetext' );
2411	2445
—	—	@@ -3052,23 +3086,74 @@
3053	3087	$sk = $wgUser->getSkin();
3054	3088	$lnk = $current
3055	3089	? wfMsgHtml( 'currentrevisionlink' )
3056		~~- : $sk->makeKnownLinkObj( $this->mTitle, wfMsgHtml( 'currentrevisionlink' ) );~~
	3090	+ : $sk->link(
	3091	+ $this->mTitle,
	3092	+ wfMsgHtml( 'currentrevisionlink' ),
	3093	+ array(),
	3094	+ array(),
	3095	+ array( 'known', 'noclasses' )
	3096	+ );
3057	3097	$curdiff = $current
3058	3098	? wfMsgHtml( 'diff' )
3059		~~- : $sk->makeKnownLinkObj( $this->mTitle, wfMsgHtml( 'diff' ), 'diff=cur&oldid='.$oldid );~~
	3099	+ : $sk->link(
	3100	+ $this->mTitle,
	3101	+ wfMsgHtml( 'diff' ),
	3102	+ array(),
	3103	+ array(
	3104	+ 'diff' => 'cur',
	3105	+ 'oldid' => $oldid
	3106	+ ),
	3107	+ array( 'known', 'noclasses' )
	3108	+ );
3060	3109	$prev = $this->mTitle->getPreviousRevisionID( $oldid ) ;
3061	3110	$prevlink = $prev
3062		~~- ? $sk->makeKnownLinkObj( $this->mTitle, wfMsgHtml( 'previousrevision' ), 'direction=prev&oldid='.$oldid )~~
	3111	+ ? $sk->link(
	3112	+ $this->mTitle,
	3113	+ wfMsgHtml( 'previousrevision' ),
	3114	+ array(),
	3115	+ array(
	3116	+ 'direction' => 'prev',
	3117	+ 'oldid' => $oldid
	3118	+ ),
	3119	+ array( 'known', 'noclasses' )
	3120	+ )
3063	3121	: wfMsgHtml( 'previousrevision' );
3064	3122	$prevdiff = $prev
3065		~~- ? $sk->makeKnownLinkObj( $this->mTitle, wfMsgHtml( 'diff' ), 'diff=prev&oldid='.$oldid )~~
	3123	+ ? $sk->link(
	3124	+ $this->mTitle,
	3125	+ wfMsgHtml( 'diff' ),
	3126	+ array(),
	3127	+ array(
	3128	+ 'diff' => 'prev',
	3129	+ 'oldid' => $oldid
	3130	+ ),
	3131	+ array( 'known', 'noclasses' )
	3132	+ )
3066	3133	: wfMsgHtml( 'diff' );
3067	3134	$nextlink = $current
3068	3135	? wfMsgHtml( 'nextrevision' )
3069		~~- : $sk->makeKnownLinkObj( $this->mTitle, wfMsgHtml( 'nextrevision' ), 'direction=next&oldid='.$oldid );~~
	3136	+ : $sk->link(
	3137	+ $this->mTitle,
	3138	+ wfMsgHtml( 'nextrevision' ),
	3139	+ array(),
	3140	+ array(
	3141	+ 'direction' => 'next',
	3142	+ 'oldid' => $oldid
	3143	+ ),
	3144	+ array( 'known', 'noclasses' )
	3145	+ );
3070	3146	$nextdiff = $current
3071	3147	? wfMsgHtml( 'diff' )
3072		~~- : $sk->makeKnownLinkObj( $this->mTitle, wfMsgHtml( 'diff' ), 'diff=next&oldid='.$oldid );~~
	3148	+ : $sk->link(
	3149	+ $this->mTitle,
	3150	+ wfMsgHtml( 'diff' ),
	3151	+ array(),
	3152	+ array(
	3153	+ 'diff' => 'next',
	3154	+ 'oldid' => $oldid
	3155	+ ),
	3156	+ array( 'known', 'noclasses' )
	3157	+ );
3073	3158
3074	3159	$cdel='';
3075	3160	if( $wgUser->isAllowed( 'deleterevision' ) ) {
—	—	@@ -3080,11 +3165,17 @@
3081	3166	// If revision was hidden from sysops
3082	3167	$cdel = wfMsgHtml( 'rev-delundel' );
3083	3168	} else {
3084		~~- $cdel = $sk->makeKnownLinkObj( $revdel,~~
	3169	+ $cdel = $sk->link(
	3170	+ $revdel,
3085	3171	wfMsgHtml('rev-delundel'),
3086		~~- 'type=revision' .~~
3087		~~- '&target=' . urlencode( $this->mTitle->getPrefixedDbkey() ) .~~
3088		~~- '&ids=' . urlencode( $oldid ) );~~
	3172	+ array(),
	3173	+ array(
	3174	+ 'type' => 'revision',
	3175	+ 'target' => urlencode( $this->mTitle->getPrefixedDbkey() ),
	3176	+ 'ids' => urlencode( $oldid )
	3177	+ ),
	3178	+ array( 'known', 'noclasses' )
	3179	+ );
3089	3180	// Bolden oversighted content
3090	3181	if( $revision->isDeleted( Revision::DELETED_RESTRICTED ) )
3091	3182	$cdel = "<strong>$cdel</strong>";
Index: trunk/phase3/includes/ImagePage.php
—	—	@@ -381,7 +381,13 @@
382	382
383	383	if( $page > 1 ) {
384	384	$label = $wgOut->parse( wfMsg( 'imgmultipageprev' ), false );
385		~~- $link = $sk->makeKnownLinkObj( $this->mTitle, $label, 'page='. ($page-1) );~~
	385	+ $link = $sk->link(
	386	+ $this->mTitle,
	387	+ $label,
	388	+ array(),
	389	+ array( 'page' => $page - 1 ),
	390	+ array( 'known', 'noclasses' )
	391	+ );
386	392	$thumb1 = $sk->makeThumbLinkObj( $this->mTitle, $this->displayImg, $link, $label, 'none',
387	393	array( 'page' => $page - 1 ) );
388	394	} else {
—	—	@@ -390,7 +396,13 @@
391	397
392	398	if( $page < $count ) {
393	399	$label = wfMsg( 'imgmultipagenext' );
394		~~- $link = $sk->makeKnownLinkObj( $this->mTitle, $label, 'page='. ($page+1) );~~
	400	+ $link = $sk->link(
	401	+ $this->mTitle,
	402	+ $label,
	403	+ array(),
	404	+ array( 'page' => $page + 1 ),
	405	+ array( 'known', 'noclasses' )
	406	+ );
395	407	$thumb2 = $sk->makeThumbLinkObj( $this->mTitle, $this->displayImg, $link, $label, 'none',
396	408	array( 'page' => $page + 1 ) );
397	409	} else {
—	—	@@ -547,7 +559,17 @@
548	560	}
549	561
550	562	# External editing link
551		~~- $elink = $sk->makeKnownLinkObj( $this->mTitle, wfMsgHtml( 'edit-externally' ), 'action=edit&externaledit=true&mode=file' );~~
	563	+ $elink = $sk->link(
	564	+ $this->mTitle,
	565	+ wfMsgHtml( 'edit-externally' ),
	566	+ array(),
	567	+ array(
	568	+ 'action' => 'edit',
	569	+ 'externaledit' => 'true',
	570	+ 'mode' => 'file'
	571	+ ),
	572	+ array( 'known', 'noclasses' )
	573	+ );
552	574	$wgOut->addHTML( '<li id="mw-imagepage-edit-external">' . $elink . ' <small>' . wfMsgExt( 'edit-externally-help', array( 'parseinline' ) ) . "</small></li>\n" );
553	575
554	576	$wgOut->addHTML( "</ul>\n" );
—	—	@@ -615,8 +637,13 @@
616	638	$count++;
617	639	if( $count <= $limit ) {
618	640	// We have not yet reached the extra one that tells us there is more to fetch
619		~~- $name = Title::makeTitle( $s->page_namespace, $s->page_title );~~
620		~~- $link = $sk->makeKnownLinkObj( $name, "" );~~
	641	+ $link = $sk->link(
	642	+ Title::makeTitle( $s->page_namespace, $s->page_title ),
	643	+ null,
	644	+ array(),
	645	+ array(),
	646	+ array( 'known', 'noclasses' )
	647	+ );
621	648	$wgOut->addHTML( "<li>{$link}</li>\n" );
622	649	}
623	650	}
—	—	@@ -643,7 +670,13 @@
644	671
645	672	$sk = $wgUser->getSkin();
646	673	foreach ( $redirects as $title ) {
647		~~- $link = $sk->makeKnownLinkObj( $title, "", "redirect=no" );~~
	674	+ $link = $sk->link(
	675	+ $title,
	676	+ null,
	677	+ array(),
	678	+ array( 'redirect' => 'no' ),
	679	+ array( 'known', 'noclasses' )
	680	+ );
648	681	$wgOut->addHTML( "<li>{$link}</li>\n" );
649	682	}
650	683	$wgOut->addHTML( "</ul></div>\n" );
—	—	@@ -667,9 +700,15 @@
668	701	$sk = $wgUser->getSkin();
669	702	foreach ( $dupes as $file ) {
670	703	$fromSrc = '';
671		~~- if( $file->isLocal() )~~
672		~~- $link = $sk->makeKnownLinkObj( $file->getTitle(), "" );~~
673		~~- else {~~
	704	+ if( $file->isLocal() ) {
	705	+ $link = $sk->link(
	706	+ $file->getTitle(),
	707	+ null,
	708	+ array(),
	709	+ array(),
	710	+ array( 'known', 'noclasses' )
	711	+ );
	712	+ } else {
674	713	$link = $sk->makeExternalLink( $file->getDescriptionUrl(),
675	714	$file->getTitle()->getPrefixedText() );
676	715	$fromSrc = wfMsg( 'shared-repo-from', $file->getRepo()->getDisplayName() );
—	—	@@ -851,13 +890,17 @@
852	891	if( $file->isDeleted(File::DELETED_FILE) ) {
853	892	$row .= wfMsgHtml('filehist-revert');
854	893	} else {
855		~~- $q = array();~~
856		~~- $q[] = 'action=revert';~~
857		~~- $q[] = 'oldimage=' . urlencode( $img );~~
858		~~- $q[] = 'wpEditToken=' . urlencode( $wgUser->editToken( $img ) );~~
859		~~- $row .= $this->skin->makeKnownLinkObj( $this->title,~~
	894	+ $row .= $this->skin->link(
	895	+ $this->title,
860	896	wfMsgHtml( 'filehist-revert' ),
861		~~- implode( '&', $q ) );~~
	897	+ array(),
	898	+ array(
	899	+ 'action' => 'revert',
	900	+ 'oldimage' => $img,
	901	+ 'wpEditToken' => $wgUser->editToken( $img )
	902	+ ),
	903	+ array( 'known', 'noclasses' )
	904	+ );
862	905	}
863	906	}
864	907	$row .= '</td>';
—	—	@@ -873,10 +916,17 @@
874	917	} elseif( $file->isDeleted(File::DELETED_FILE) ) {
875	918	$revdel = SpecialPage::getTitleFor( 'Revisiondelete' );
876	919	# Make a link to review the image
877		~~- $url = $this->skin->makeKnownLinkObj( $revdel, $wgLang->timeAndDate( $timestamp, true ),~~
878		~~- "target=".$wgTitle->getPrefixedUrl().~~
879		~~- "&file=" . urlencode( $img ) .~~
880		~~- "&token=" . urlencode( $wgUser->editToken( $img ) ) );~~
	920	+ $url = $this->skin->link(
	921	+ $revdel,
	922	+ $wgLang->timeAndDate( $timestamp, true ),
	923	+ array(),
	924	+ array(
	925	+ 'target' => $wgTitle->getPrefixedUrl(),
	926	+ 'file' => $img,
	927	+ 'token' => $wgUser->editToken( $img )
	928	+ ),
	929	+ array( 'known', 'noclasses' )
	930	+ );
881	931	$row .= '<span class="history-deleted">'.$url.'</span>';
882	932	} elseif( $file->isMissing() ) {
883	933	# Don't link to missing files
Index: trunk/phase3/includes/EditPage.php
—	—	@@ -1282,15 +1282,18 @@
1283	1283	$wgOut->addHTML( "</div>\n" );
1284	1284	}
1285	1285
1286		~~- $q = 'action='.$this->action;~~
1287		~~- #if ( "no" == $redirect ) { $q .= "&redirect=no"; }~~
1288		~~- $action = $wgTitle->escapeLocalURL( $q );~~
	1286	+ $action = $wgTitle->escapeLocalURL( array( 'action' => $this->action ) );
1289	1287
1290	1288	$summary = wfMsgExt( 'summary', 'parseinline' );
1291	1289	$subject = wfMsgExt( 'subject', 'parseinline' );
1292	1290
1293		~~- $cancel = $sk->makeKnownLink( $wgTitle->getPrefixedText(),~~
1294		~~- wfMsgExt('cancel', array('parseinline')) );~~
	1291	+ $cancel = $sk->link(
	1292	+ $wgTitle->getPrefixedText(),
	1293	+ wfMsgExt( 'cancel', array( 'parseinline' ) ),
	1294	+ array(),
	1295	+ array(),
	1296	+ array( 'known', 'noclasses' )
	1297	+ );
1295	1298	$separator = wfMsgExt( 'pipe-separator' , 'escapenoentities' );
1296	1299	$edithelpurl = Skin::makeInternalOrExternalUrl( wfMsgForContent( 'edithelppage' ));
1297	1300	$edithelp = '<a target="helpwindow" href="'.$edithelpurl.'">'.
—	—	@@ -1674,7 +1677,11 @@
1675	1678	function doLivePreviewScript() {
1676	1679	global $wgOut, $wgTitle;
1677	1680	$wgOut->addScriptFile( 'preview.js' );
1678		~~- $liveAction = $wgTitle->getLocalUrl( "action={$this->action}&wpPreview=true&live=true" );~~
	1681	+ $liveAction = $wgTitle->getLocalUrl( array(
	1682	+ 'action' => $this->action,
	1683	+ 'wpPreview' => 'true',
	1684	+ 'live' => 'true'
	1685	+ ) );
1679	1686	return "return !lpDoPreview(" .
1680	1687	"editform.wpTextbox1.value," .
1681	1688	'"' . $liveAction . '"' . ")";
—	—	@@ -1864,7 +1871,13 @@
1865	1872	$skin = $wgUser->getSkin();
1866	1873
1867	1874	$loginTitle = SpecialPage::getTitleFor( 'Userlogin' );
1868		~~- $loginLink = $skin->makeKnownLinkObj( $loginTitle, wfMsgHtml( 'loginreqlink' ), 'returnto=' . $wgTitle->getPrefixedUrl() );~~
	1875	+ $loginLink = $skin->link(
	1876	+ $loginTitle,
	1877	+ wfMsgHtml( 'loginreqlink' ),
	1878	+ array(),
	1879	+ array( 'returnto' => $wgTitle->getPrefixedUrl() ),
	1880	+ array( 'known', 'noclasses' )
	1881	+ );
1869	1882
1870	1883	$wgOut->setPageTitle( wfMsg( 'whitelistedittitle' ) );
1871	1884	$wgOut->setRobotPolicy( 'noindex,nofollow' );
Index: trunk/phase3/includes/ChangesList.php
—	—	@@ -160,12 +160,32 @@
161	161	# Diff
162	162	$s .= '(' . $this->message['diff'] . ') (';
163	163	# Hist
164		~~- $s .= $this->skin->makeKnownLinkObj( $rc->getMovedToTitle(), $this->message['hist'],~~
165		~~- 'action=history' ) . ') . . ';~~
	164	+ $s .= $this->skin->link(
	165	+ $rc->getMovedToTitle(),
	166	+ $this->message['hist'],
	167	+ array(),
	168	+ array( 'action' => 'history' ),
	169	+ array( 'known', 'noclasses' )
	170	+ ) . ') . . ';
166	171	# "[[x]] moved to [[y]]"
167	172	$msg = ( $rc->mAttribs['rc_type'] == RC_MOVE ) ? '1movedto2' : '1movedto2_redir';
168		~~- $s .= wfMsg( $msg, $this->skin->makeKnownLinkObj( $rc->getTitle(), '', 'redirect=no' ),~~
169		~~- $this->skin->makeKnownLinkObj( $rc->getMovedToTitle(), '' ) );~~
	173	+ $s .= wfMsg(
	174	+ $msg,
	175	+ $this->skin->link(
	176	+ $rc->getTitle(),
	177	+ null,
	178	+ array(),
	179	+ array( 'redirect' => 'no' ),
	180	+ array( 'known', 'noclasses' )
	181	+ ),
	182	+ $this->skin->link(
	183	+ $rc->getMovedToTitle(),
	184	+ null,
	185	+ array(),
	186	+ array(),
	187	+ array( 'known', 'noclasses' )
	188	+ )
	189	+ );
170	190	}
171	191
172	192	protected function insertDateHeader( &$s, $rc_timestamp ) {
—	—	@@ -184,7 +204,13 @@
185	205
186	206	protected function insertLog( &$s, $title, $logtype ) {
187	207	$logname = LogPage::logName( $logtype );
188		~~- $s .= '(' . $this->skin->makeKnownLinkObj($title, $logname ) . ')';~~
	208	+ $s .= '(' . $this->skin->link(
	209	+ $title,
	210	+ $logname,
	211	+ array(),
	212	+ array(),
	213	+ array( 'known', 'noclasses' )
	214	+ ) . ')';
189	215	}
190	216
191	217	protected function insertDiffHist( &$s, &$rc, $unpatrolled ) {
—	—	@@ -194,21 +220,36 @@
195	221	} else if( !$this->userCan($rc,Revision::DELETED_TEXT) ) {
196	222	$diffLink = $this->message['diff'];
197	223	} else {
198		~~- $rcidparam = $unpatrolled ? array( 'rcid' => $rc->mAttribs['rc_id'] ) : array();~~
199		~~- $diffLink = $this->skin->makeKnownLinkObj( $rc->getTitle(), $this->message['diff'],~~
200		~~- wfArrayToCGI( array(~~
201		~~- 'curid' => $rc->mAttribs['rc_cur_id'],~~
202		~~- 'diff' => $rc->mAttribs['rc_this_oldid'],~~
203		~~- 'oldid' => $rc->mAttribs['rc_last_oldid'] ),~~
204		~~- $rcidparam ),~~
205		~~- '', '', ' tabindex="'.$rc->counter.'"');~~
	224	+ $query = array(
	225	+ 'curid' => $rc->mAttribs['rc_cur_id'],
	226	+ 'diff' => $rc->mAttribs['rc_this_oldid'],
	227	+ 'oldid' => $rc->mAttribs['rc_last_oldid']
	228	+ );
	229	+
	230	+ if( $unpatrolled ) {
	231	+ $query['rcid'] = $rc->mAttribs['rc_id'];
	232	+ };
	233	+
	234	+ $diffLink = $this->skin->link(
	235	+ $rc->getTitle(),
	236	+ $this->message['diff'],
	237	+ array( 'tabindex' => $rc->counter ),
	238	+ $query,
	239	+ array( 'known', 'noclasses' )
	240	+ );
206	241	}
207	242	$s .= '('.$diffLink.') (';
208	243	# History link
209		~~- $s .= $this->skin->makeKnownLinkObj( $rc->getTitle(), $this->message['hist'],~~
210		~~- wfArrayToCGI( array(~~
	244	+ $s .= $this->skin->link(
	245	+ $rc->getTitle(),
	246	+ $this->message['hist'],
	247	+ array(),
	248	+ array(
211	249	'curid' => $rc->mAttribs['rc_cur_id'],
212		~~- 'action' => 'history' ) ) );~~
	250	+ 'action' => 'history'
	251	+ ),
	252	+ array( 'known', 'noclasses' )
	253	+ );
213	254	$s .= ') . . ';
214	255	}
215	256
—	—	@@ -216,13 +257,29 @@
217	258	global $wgContLang;
218	259	# If it's a new article, there is no diff link, but if it hasn't been
219	260	# patrolled yet, we need to give users a way to do so
220		~~- $params = ( $unpatrolled && $rc->mAttribs['rc_type'] == RC_NEW ) ?~~
221		~~- 'rcid='.$rc->mAttribs['rc_id'] : '';~~
	261	+ $params = array();
	262	+
	263	+ if ( $unpatrolled && $rc->mAttribs['rc_type'] == RC_NEW ) {
	264	+ $params['rcid'] = $rc->mAttribs['rc_id'];
	265	+ }
	266	+
222	267	if( $this->isDeleted($rc,Revision::DELETED_TEXT) ) {
223		~~- $articlelink = $this->skin->makeKnownLinkObj( $rc->getTitle(), '', $params );~~
224		~~- $articlelink = '<span class="history-deleted">'.$articlelink.'</span>';~~
	268	+ $articlelink = $this->skin->link(
	269	+ $rc->getTitle(),
	270	+ null,
	271	+ array(),
	272	+ $params,
	273	+ array( 'known', 'noclasses' )
	274	+ );
	275	+ $articlelink = '<span class="history-deleted">' . $articlelink . '</span>';
225	276	} else {
226		~~- $articlelink = ' '. $this->skin->makeKnownLinkObj( $rc->getTitle(), '', $params );~~
	277	+ $articlelink = ' '. $this->skin->link(
	278	+ $rc->getTitle(),
	279	+ null,
	280	+ array(),
	281	+ $params,
	282	+ array( 'known', 'noclasses' )
	283	+ );
227	284	}
228	285	# Bolden pages watched by this user
229	286	if( $watched ) {
—	—	@@ -332,7 +389,7 @@
333	390	}
334	391	}
335	392
336		~~- protected function maybeWatchedLink( $link, $watched=false ) {~~
	393	+ protected function maybeWatchedLink( $link, $watched = false ) {
337	394	if( $watched ) {
338	395	return '<strong class="mw-watched">' . $link . '</strong>';
339	396	} else {
—	—	@@ -569,7 +626,7 @@
570	627	$rc->timestamp = $time;
571	628	$rc->numberofWatchingusers = $baseRC->numberofWatchingusers;
572	629
573		~~- # Make "cur" and "diff" links. Don't use link(), it's too slow if~~
	630	+ # Make "cur" and "diff" links. Do not use link(), it is too slow if
574	631	# called too many times (50% of CPU time on RecentChanges!).
575	632	if( $rc->unpatrolled ) {
576	633	$rcIdQuery = array( 'rcid' => $rc_id );
—	—	@@ -600,7 +657,7 @@
601	658
602	659	# Make "last" link
603	660	if( !$showdifflinks \|\| !$rc_last_oldid ) {
604		~~- $lastLink = $this->message['last'];~~
	661	+ $lastLink = $this->message['last'];
605	662	} else if( $rc_type == RC_LOG \|\| $rc_type == RC_MOVE \|\| $rc_type == RC_MOVE_OVER_REDIRECT ) {
606	663	$lastLink = $this->message['last'];
607	664	} else {
—	—	@@ -741,7 +798,7 @@
742	799
743	800	$r .= $wgContLang->getDirMark();
744	801
745		~~- $curIdEq = 'curid=' . $curId;~~
	802	+ $queryParams['curid'] = $curId;
746	803	# Changes message
747	804	$n = count($block);
748	805	static $nchanges = array();
—	—	@@ -757,8 +814,17 @@
758	815	} else if( $isnew ) {
759	816	$r .= $nchanges[$n];
760	817	} else {
761		~~- $r .= $this->skin->makeKnownLinkObj( $block[0]->getTitle(),~~
762		~~- $nchanges[$n], $curIdEq."&diff=$currentRevision&oldid=$oldid" );~~
	818	+ $params = $queryParams;
	819	+ $params['diff'] = $currentRevision;
	820	+ $params['oldid'] = $oldid;
	821	+
	822	+ $r .= $this->skin->link(
	823	+ $block[0]->getTitle(),
	824	+ $nchanges[$n],
	825	+ array(),
	826	+ $params,
	827	+ array( 'known', 'noclasses' )
	828	+ );
763	829	}
764	830	}
765	831
—	—	@@ -768,8 +834,17 @@
769	835	} else if( $namehidden \|\| !$block[0]->getTitle()->exists() ) {
770	836	$r .= $this->message['semicolon-separator'] . $this->message['hist'] . ')';
771	837	} else {
772		~~- $r .= $this->message['semicolon-separator'] . $this->skin->makeKnownLinkObj( $block[0]->getTitle(),~~
773		~~- $this->message['hist'], $curIdEq . '&action=history' ) . ')';~~
	838	+ $params = $queryParams;
	839	+ $params['action'] = 'history';
	840	+
	841	+ $r .= $this->message['semicolon-separator'] .
	842	+ $this->skin->link(
	843	+ $block[0]->getTitle(),
	844	+ $this->message['hist'],
	845	+ array(),
	846	+ $params,
	847	+ array( 'known', 'noclasses' )
	848	+ ) . ')';
774	849	}
775	850	$r .= ' . . ';
776	851
—	—	@@ -817,10 +892,12 @@
818	893	$r .= $this->recentChangesFlags( $rc_new, $rc_minor, $rcObj->unpatrolled, ' ', $rc_bot );
819	894	$r .= ' </tt></td><td valign="top">';
820	895
821		~~- $o = '';~~
	896	+ $params = $queryParams;
	897	+
822	898	if( $rc_this_oldid != 0 ) {
823		~~- $o = 'oldid='.$rc_this_oldid;~~
	899	+ $params['oldid'] = $rc_this_oldid;
824	900	}
	901	+
825	902	# Log timestamp
826	903	if( $rc_type == RC_LOG ) {
827	904	$link = '<tt>'.$rcObj->timestamp.'</tt> ';
—	—	@@ -828,10 +905,18 @@
829	906	} else if( !ChangesList::userCan($rcObj,Revision::DELETED_TEXT) ) {
830	907	$link = '<span class="history-deleted"><tt>'.$rcObj->timestamp.'</tt></span> ';
831	908	} else {
832		~~- $rcIdEq = ($rcObj->unpatrolled && $rc_type == RC_NEW) ?~~
833		~~- '&rcid='.$rcObj->mAttribs['rc_id'] : '';~~
834		~~- $link = '<tt>'.$this->skin->makeKnownLinkObj( $rcObj->getTitle(),~~
835		~~- $rcObj->timestamp, $curIdEq.'&'.$o.$rcIdEq ).'</tt>';~~
	909	+ if ( $rcObj->unpatrolled && $rc_type == RC_NEW) {
	910	+ $params['rcid'] = $rcObj->mAttribs['rc_id'];
	911	+ }
	912	+
	913	+ $link = '<tt>' .
	914	+ $this->skin->link(
	915	+ $rcObj->getTitle(),
	916	+ $rcObj->timestamp,
	917	+ array(),
	918	+ $params,
	919	+ array( 'known', 'noclasses' )
	920	+ ) . '</tt>';
836	921	if( $this->isDeleted($rcObj,Revision::DELETED_TEXT) )
837	922	$link = '<span class="history-deleted">'.$link.'</span> ';
838	923	}
—	—	@@ -938,7 +1023,7 @@
939	1024	// that explicitly initializes variables.
940	1025	$classes = array(); // TODO implement
941	1026	extract( $rcObj->mAttribs );
942		~~- $curIdEq = "curid={$rc_cur_id}";~~
	1027	+ $query['curid'] = $rc_cur_id;
943	1028
944	1029	$r = '<table cellspacing="0" cellpadding="0" border="0" style="background: none"><tr>';
945	1030	$r .= '<td valign="top" style="white-space: nowrap"><tt>' . $this->spacerArrow() . ' ';
—	—	@@ -953,15 +1038,27 @@
954	1039	if( $rc_log_type ) {
955	1040	$logtitle = Title::newFromText( "Log/$rc_log_type", NS_SPECIAL );
956	1041	$logname = LogPage::logName( $rc_log_type );
957		~~- $r .= '(' . $this->skin->makeKnownLinkObj($logtitle, $logname ) . ')';~~
	1042	+ $r .= '(' . $this->skin->link(
	1043	+ $logtitle,
	1044	+ $logname,
	1045	+ array(),
	1046	+ array(),
	1047	+ array( 'known', 'noclasses' )
	1048	+ ) . ')';
958	1049	} else {
959	1050	$this->insertArticleLink( $r, $rcObj, $rcObj->unpatrolled, $rcObj->watched );
960	1051	}
961	1052	# Diff and hist links
962	1053	if ( $rc_type != RC_LOG ) {
963	1054	$r .= ' ('. $rcObj->difflink . $this->message['semicolon-separator'];
964		~~- $r .= $this->skin->makeKnownLinkObj( $rcObj->getTitle(), $this->message['hist'],~~
965		~~- $curIdEq.'&action=history' ) . ')';~~
	1055	+ $query['action'] = 'history';
	1056	+ $r .= $this->skin->link(
	1057	+ $rcObj->getTitle(),
	1058	+ $this->message['hist'],
	1059	+ array(),
	1060	+ $query,
	1061	+ array( 'known', 'noclasses' )
	1062	+ ) . ')';
966	1063	}
967	1064	$r .= ' . . ';
968	1065	# Character diff
Index: trunk/phase3/includes/FileDeleteForm.php
—	—	@@ -251,7 +251,16 @@
252	252	global $wgOut, $wgUser;
253	253	$wgOut->setPageTitle( wfMsg( 'filedelete', $this->title->getText() ) );
254	254	$wgOut->setRobotPolicy( 'noindex,nofollow' );
255		~~- $wgOut->setSubtitle( wfMsg( 'filedelete-backlink', $wgUser->getSkin()->makeKnownLinkObj( $this->title ) ) );~~
	255	+ $wgOut->setSubtitle( wfMsg(
	256	+ 'filedelete-backlink',
	257	+ $wgUser->getSkin()->link(
	258	+ $this->title,
	259	+ null,
	260	+ array(),
	261	+ array(),
	262	+ array( 'known', 'noclasses' )
	263	+ )
	264	+ ) );
256	265	}
257	266
258	267	/**
—	—	@@ -284,11 +293,10 @@
285	294	* @return string
286	295	*/
287	296	private function getAction() {
288		~~- $q = array();~~
289		~~- $q[] = 'action=delete';~~
	297	+ $q['action'] = 'delete';
290	298	if( $this->oldimage )
291		~~- $q[] = 'oldimage=' . urlencode( $this->oldimage );~~
292		~~- return $this->title->getLocalUrl( implode( '&', $q ) );~~
	299	+ $q['oldimage'] = $this->oldimage;
	300	+ return $this->title->getLocalUrl( $q );
293	301	}
294	302
295	303	/**
—	—	@@ -299,5 +307,4 @@
300	308	private function getTimestamp() {
301	309	return $this->oldfile->getTimestamp();
302	310	}
303		-
304	311	}

Follow-up revisions

Revision	Commit summary	Author	Date
r51561	Follow-up to r51559 per CR: initialise $q to prevent PHP notice.	siebrand	10:34, 7 June 2009
r51568	* replace some use of deprecated makeKnownLinkObj() by link() in core...	siebrand	15:02, 7 June 2009
r51572	* replace use of deprecated makeKnownLinkObj() by linkKnown() in core special...	siebrand	18:45, 7 June 2009
r51586	Replace a few remaining uses of deprecated makeKnownLinkObj() by link[Known](...	siebrand	10:07, 8 June 2009
r51588	(bug 19126) Follow-up to r51559: replace a period with a comma to get expecte...	siebrand	12:46, 8 June 2009
r51820	* replace deprecated makeBrokenLinkObj() by LinkHolderArray.php()...	siebrand	12:39, 13 June 2009
r51821	* deprecated makeMediaLink() to makeMediaLinkObj...	siebrand	13:41, 13 June 2009
r51894	Fix double escaping issues. Related to r51559 and friends.	siebrand	12:32, 15 June 2009
r51904	Fix up r51559: Linker::link() expects Title objects, not strings	catrope	13:50, 15 June 2009
r58390	Fix for r51559: removed double urlencode()'ing	ialex	19:42, 31 October 2009

Comments

#Comment by Siebrand (talk | contribs) 22:46, 6 June 2009

Fairly large commit. Please review, because of high impact.

#Comment by Werdna (talk | contribs) 23:02, 6 June 2009

A lot of the statements should be split into multiple lines instead of broken.
It isn't clear that array( 'known', 'noclasses' ) is appropriate in all cases.
Second to last hunk (getAction) will introduce PHP warnings, because $q will be undefined. Also includes some logic changes, were they intentional? Not mentioned in the commit message.

+		# Make "cur" and "diff" links.  Do not use link(), it is too slow if
 		# called too many times (50% of CPU time on RecentChanges!).

I assume you read this comment. In either case, I bet that would be fixed with the known, noclasses thing.

-			if( $file->isLocal() )
-				$link = $sk->makeKnownLinkObj( $file->getTitle(), "" );
-			else {
+			if( $file->isLocal() ) {
+				$link = $sk->link(
+					$file->getTitle(),
+					null,

Don't use 'null' to mean an empty string. Use an empty string.

#Comment by Siebrand (talk | contribs) 10:04, 7 June 2009

I'll answer your questions and remarks in different replies. I think that will allow for a better overview if there are replies to it.

Split in multiple lines instead of broken: Not sure I understand. I have tried to keep the previous code structure in place as much as possible. Can you prove an example? I'll try to do better in following commits and fix where possible/needed.

#Comment by Werdna (talk | contribs) 10:27, 7 June 2009

It's more a comment on the code than on you, but what I mean is that this:

+		$wgOut->setSubtitle( wfMsg(
+			'filerevert-backlink',
+			$wgUser->getSkin()->link(
+				$this->title,
+				null,
+				array(),
+				array(),
+				array( 'known', 'noclasses' )
+			)
+		) );

should be this:

$sk = $wgUser->getSkin();
$backLink = $sk->link( $this->title, null, array(), array(), array( 'known', 'noclasses' );
$msg = wfMsg( 'filerevert-backlink', $backLink );
$wgOut->setSubtitle( $msg );
(broken where appropriate)

#Comment by Siebrand (talk | contribs) 10:08, 7 June 2009

Need for "array( 'known', 'noclasses' )": this is what Linker::makeKnownLinkObj() does. I agree with you that this may not be necessary in every case, but because I am unable to assess this for code that is this fundamental to MediaWiki I decided to not change the current behaviour.

From Linker::makeKnownLinkObj:

		$ret = $this->link( $title, "$prefix$text$inside", $attribs, $query,
			array( 'known', 'noclasses' ) ) . $trail;

What do you suggest is done?

#Comment by Werdna (talk | contribs) 10:25, 7 June 2009

Leave it for now, it's just a bit ugly.

#Comment by Siebrand (talk | contribs) 10:11, 7 June 2009

FileDeleteForm.php, $q['action'] = 'delete';: So "$q = array();" is required here? Please confirm, I'll add it. I thought it was possible to assign a key value pair to a previously uninitialised variable.

#Comment by Werdna (talk | contribs) 10:24, 7 June 2009

Yes, it's possible but it causes a PHP notice.

#Comment by Siebrand (talk | contribs) 10:34, 7 June 2009

Fixed in r51561.

#Comment by Siebrand (talk | contribs) 10:15, 7 June 2009

Did I read the comment? Yes, although I didn't really understand it. No changes ware made in EnhancedChangesList::recentChangesLine() (ChangesList.php l.548-l.702) except for two indentation fixes and a trivial update in the comment. Would you like me to change/revert anything here?

Thanks for the review!

#Comment by Siebrand (talk | contribs) 10:17, 7 June 2009

Do not use 'null': Linker::public function link( $target, $text = null, $customAttribs = array(), $query = array(), $options = array() )

Why should I use "''" if the default for the text parameter in Linker::link() *is* null?

#Comment by Werdna (talk | contribs) 10:24, 7 June 2009

Ah, it's crappy coding on the part of whoever wrote the original, not you :)

#Comment by Tim Starling (talk | contribs) 03:19, 11 June 2009

I assume this is untested?

#Comment by Siebrand (talk | contribs) 07:58, 11 June 2009

Been running ever since commit at translatewiki.net. Haven't gotten any reports on weirdness so far. ~~~~

Status & tagging log

02:38, 15 June 2009 Tim Starling (talk | contribs) changed the status of r51559 [removed: new added: ok]