r95002 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r95001‎ | r95002 | r95003 >
Date:14:46, 19 August 2011
Author:catrope
Status:resolved (Comments)
Tags:
Comment:
Use canonical URLs (introduced in r94995) for all URLs in e-mails
Modified paths:
  • /trunk/phase3/includes/User.php (modified) (history)
  • /trunk/phase3/includes/UserMailer.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/User.php
@@ -3315,14 +3315,14 @@
33163316 * @return String Formatted URL
33173317 */
33183318 protected function getTokenUrl( $page, $token ) {
3319 - global $wgArticlePath;
3320 - return wfExpandUrl(
 3319+ global $wgCanonicalServer, $wgArticlePath;
 3320+
 3321+ return $wgCanonicalServer .
33213322 str_replace(
33223323 '$1',
33233324 "Special:$page/$token",
3324 - $wgArticlePath ),
3325 - PROTO_HTTP
3326 - );
 3325+ $wgArticlePath
 3326+ );
33273327 }
33283328
33293329 /**
Index: trunk/phase3/includes/UserMailer.php
@@ -523,7 +523,7 @@
524524 $keys = array();
525525
526526 if ( $this->oldid ) {
527 - $difflink = wfExpandUrl( $this->title->getFullUrl( 'diff=0&oldid=' . $this->oldid ), PROTO_HTTP );
 527+ $difflink = $this->title->getCanonicalUrl( 'diff=0&oldid=' . $this->oldid );
528528 $keys['$NEWPAGE'] = wfMsgForContent( 'enotif_lastvisited', $difflink );
529529 $keys['$OLDID'] = $this->oldid;
530530 $keys['$CHANGEDORCREATED'] = wfMsgForContent( 'changed' );
@@ -540,17 +540,17 @@
541541 * revision.
542542 */
543543 $keys['$NEWPAGE'] = wfMsgForContent( 'enotif_lastdiff',
544 - wfExpandUrl( $this->title->getFullURL( "oldid={$this->oldid}&diff=next" ), PROTO_HTTP ) );
 544+ $this->title->getCanonicalUrl( "oldid={$this->oldid}&diff=next" ) );
545545 }
546546
547547 $body = strtr( $body, $keys );
548548 $pagetitle = $this->title->getPrefixedText();
549549 $keys['$PAGETITLE'] = $pagetitle;
550 - $keys['$PAGETITLE_URL'] = wfExpandUrl( $this->title->getFullUrl(), PROTO_HTTP );
 550+ $keys['$PAGETITLE_URL'] = $this->title->getCanonicalUrl();
551551
552552 $keys['$PAGEMINOREDIT'] = $medit;
553553 $keys['$PAGESUMMARY'] = $summary;
554 - $keys['$UNWATCHURL'] = wfExpandUrl( $this->title->getFullUrl( 'action=unwatch' ), PROTO_HTTP );
 554+ $keys['$UNWATCHURL'] = $this->title->getCanonicalUrl( 'action=unwatch' );
555555
556556 $subject = strtr( $subject, $keys );
557557
@@ -585,10 +585,10 @@
586586 $subject = str_replace( '$PAGEEDITOR', $name, $subject );
587587 $keys['$PAGEEDITOR'] = $name;
588588 $emailPage = SpecialPage::getSafeTitleFor( 'Emailuser', $name );
589 - $keys['$PAGEEDITOR_EMAIL'] = wfExpandUrl( $emailPage->getFullUrl(), PROTO_HTTP );
 589+ $keys['$PAGEEDITOR_EMAIL'] = $emailPage->getCanonicalUrl();
590590 }
591591 $userPage = $editor->getUserPage();
592 - $keys['$PAGEEDITOR_WIKI'] = wfExpandUrl( $userPage->getFullUrl(), PROTO_HTTP );
 592+ $keys['$PAGEEDITOR_WIKI'] = $userPage->getCanonicalUrl();
593593 $body = strtr( $body, $keys );
594594 $body = wordwrap( $body, 72 );
595595

Follow-up revisions

RevisionCommit summaryAuthorDate
r955051.17wmf1: Merge a truckload of HTTPS / prot rel URL fixes: r93847, r94990, r9...catrope19:32, 25 August 2011
r95894Followup r95002: unbreak getTokenUrl() by using a simpler hack that doesn't i...catrope18:08, 31 August 2011
r964751.18: MFT r94737, r94990, r95000, r95001, r95002, r95006, r95007, r95010, r95...catrope19:37, 7 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r94995Per CR on r44412 and my promise in the commit summary of r94990, stop abusing...catrope11:23, 19 August 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   17:45, 31 August 2011

Looks like this causes bugzilla:30647 -- this code is mixing $wgCanonicalServer (eg 'http://en.wikipedia.org/' on secure.wikimedia.org) with $wgArticlePath (eg '/wikipedia/en/wiki/$1' on secure.wikimedia.org).

#Comment by Catrope (talk | contribs)   17:47, 31 August 2011

Yup. I'd already determined that this was the culprit and put it on my TODO list. Need to find a better hack to preserve Special: , maybe Title::makeTitle( NS_MAIN, 'Special:Foo' )->getCanonicalUrl()

#Comment by Catrope (talk | contribs)   18:08, 31 August 2011

Turns out that worked, fixed in r95894.

#Comment by Brion VIBBER (talk | contribs)   18:41, 31 August 2011

Resolution looks good, looks ok in my local testing.

Status & tagging log