r41333 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r41332‎ | r41333 | r41334 >
Date:02:35, 28 September 2008
Author:tstarling
Status:old (Comments)
Tags:
Comment:
* Added the ability to set the target attribute on external links with $wgExternalLinkTarget
* Removed the namespace parameter from Linker::makeExternalLink(), added a generic associative array of attributes instead. Let the Parser decide whether to use rel=nofollow.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/Linker.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)
  • /trunk/phase3/includes/parser/ParserOptions.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser_OldPP.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES
——@@ -142,6 +142,8 @@
143143 * Improved upload file type detection for OpenDocument formats
144144 * (bug 15739) Add $wgArticlePathForCurid to make links with only curid=# as the
145145 query string use the article path, rather than the script path
 146+* Added the ability to set the target attribute on external links with
 147+ $wgExternalLinkTarget
146148
147149
148150 === Bug fixes in 1.14 ===
Index: trunk/phase3/includes/DefaultSettings.php
——@@ -2883,6 +2883,11 @@
28842884 $wgSearchForwardUrl = null;
28852885
28862886 /**
 2887+ * Set a default target for external links, e.g. _blank to pop up a new window
 2888+ */
 2889+$wgExternalLinkTarget = false;
 2890+
 2891+/**
28872892 * If true, external URL links in wiki text will be given the
28882893 * rel="nofollow" attribute as a hint to search engines that
28892894 * they should not be followed for ranking purposes as they
Index: trunk/phase3/includes/parser/ParserOptions.php
——@@ -30,6 +30,7 @@
3131 var $mTemplateCallback; # Callback for template fetching
3232 var $mEnableLimitReport; # Enable limit report in an HTML comment on output
3333 var $mTimestamp; # Timestamp used for {{CURRENTDAY}} etc.
 34+ var $mExternalLinkTarget; # Target attribute for external links
3435
3536 var $mUser; # Stored user object, just used to initialise the skin
3637
——@@ -52,6 +53,7 @@
5354 function getTemplateCallback() { return $this->mTemplateCallback; }
5455 function getEnableLimitReport() { return $this->mEnableLimitReport; }
5556 function getCleanSignatures() { return $this->mCleanSignatures; }
 57+ function getExternalLinkTarget() { return $this->mExternalLinkTarget; }
5658
5759 function getSkin() {
5860 if ( !isset( $this->mSkin ) ) {
——@@ -96,6 +98,7 @@
9799 function enableLimitReport( $x = true ) { return wfSetVar( $this->mEnableLimitReport, $x ); }
98100 function setTimestamp( $x ) { return wfSetVar( $this->mTimestamp, $x ); }
99101 function setCleanSignatures( $x ) { return wfSetVar( $this->mCleanSignatures, $x ); }
 102+ function setExternalLinkTarget( $x ) { return wfSetVar( $this->mExternalLinkTarget, $x ); }
100103
101104 function __construct( $user = null ) {
102105 $this->initialiseFromUser( $user );
——@@ -114,6 +117,7 @@
115118 global $wgUseTeX, $wgUseDynamicDates, $wgInterwikiMagic, $wgAllowExternalImages;
116119 global $wgAllowExternalImagesFrom, $wgEnableImageWhitelist, $wgAllowSpecialInclusion, $wgMaxArticleSize;
117120 global $wgMaxPPNodeCount, $wgMaxTemplateDepth, $wgMaxPPExpandDepth, $wgCleanSignatures;
 121+ global $wgExternalLinkTarget;
118122 $fname = 'ParserOptions::initialiseFromUser';
119123 wfProfileIn( $fname );
120124 if ( !$userInput ) {
——@@ -151,6 +155,7 @@
152156 $this->mTemplateCallback = array( 'Parser', 'statelessFetchTemplate' );
153157 $this->mEnableLimitReport = false;
154158 $this->mCleanSignatures = $wgCleanSignatures;
 159+ $this->mExternalLinkTarget = $wgExternalLinkTarget;
155160 wfProfileOut( $fname );
156161 }
157162 }
Index: trunk/phase3/includes/parser/Parser_OldPP.php
——@@ -1353,7 +1353,7 @@
13541354 # This means that users can paste URLs directly into the text
13551355 # Funny characters like &ouml; aren't valid in URLs anyway
13561356 # This was changed in August 2004
1357 - $s .= $sk->makeExternalLink( $url, $text, false, $linktype, $this->mTitle->getNamespace() ) . $dtrail . $trail;
 1357+ $s .= $sk->makeExternalLink( $url, $text, false, $linktype, $this->getExternalLinkAttribs() ) . $dtrail . $trail;
13581358
13591359 # Register link in the output object.
13601360 # Replace unnecessary URL escape codes with the referenced character
——@@ -1366,6 +1366,19 @@
13671367 return $s;
13681368 }
13691369
 1370+ function getExternalLinkAttribs() {
 1371+ $attribs = array();
 1372+ global $wgNoFollowLinks, $wgNoFollowNsExceptions;
 1373+ $ns = $this->mTitle->getNamespace();
 1374+ if( $wgNoFollowLinks && !in_array($ns, $wgNoFollowNsExceptions) ) {
 1375+ $attribs['rel'] = 'nofollow';
 1376+ }
 1377+ if ( $this->mOptions->getExternalLinkTarget() ) {
 1378+ $attribs['target'] = $this->mOptions->getExternalLinkTarget();
 1379+ }
 1380+ return $attribs;
 1381+ }
 1382+
13701383 /**
13711384 * Replace anything that looks like a URL with a link
13721385 * @private
——@@ -1432,7 +1445,8 @@
14331446 $text = $this->maybeMakeExternalImage( $url );
14341447 if ( $text === false ) {
14351448 # Not an image, make a link
1436 - $text = $sk->makeExternalLink( $url, $wgContLang->markNoConversion($url), true, 'free', $this->mTitle->getNamespace() );
 1449+ $text = $sk->makeExternalLink( $url, $wgContLang->markNoConversion($url), true, 'free',
 1450+ $this->getExternalLinkAttribs() );
14371451 # Register it in the output object...
14381452 # Replace unnecessary URL escape codes with their equivalent characters
14391453 $pasteurized = self::replaceUnusualEscapes( $url );
Index: trunk/phase3/includes/parser/Parser.php
——@@ -1118,7 +1118,8 @@
11191119 $text = $this->maybeMakeExternalImage( $url );
11201120 if ( $text === false ) {
11211121 # Not an image, make a link
1122 - $text = $sk->makeExternalLink( $url, $wgContLang->markNoConversion($url), true, 'free', $this->mTitle->getNamespace() );
 1122+ $text = $sk->makeExternalLink( $url, $wgContLang->markNoConversion($url), true, 'free',
 1123+ $this->getExternalLinkAttribs() );
11231124 # Register it in the output object...
11241125 # Replace unnecessary URL escape codes with their equivalent characters
11251126 $pasteurized = self::replaceUnusualEscapes( $url );
——@@ -1393,11 +1394,18 @@
13941395
13951396 $url = Sanitizer::cleanUrl( $url );
13961397
 1398+ if ( $this->mOptions->mExternalLinkTarget ) {
 1399+ $attribs = array( 'target' => $this->mOptions->mExternalLinkTarget );
 1400+ } else {
 1401+ $attribs = array();
 1402+ }
 1403+
13971404 # Use the encoded URL
13981405 # This means that users can paste URLs directly into the text
13991406 # Funny characters like &ouml; aren't valid in URLs anyway
14001407 # This was changed in August 2004
1401 - $s .= $sk->makeExternalLink( $url, $text, false, $linktype, $this->mTitle->getNamespace() ) . $dtrail . $trail;
 1408+ $s .= $sk->makeExternalLink( $url, $text, false, $linktype, $this->getExternalLinkAttribs() )
 1409+ . $dtrail . $trail;
14021410
14031411 # Register link in the output object.
14041412 # Replace unnecessary URL escape codes with the referenced character
——@@ -1410,6 +1418,20 @@
14111419 return $s;
14121420 }
14131421
 1422+ function getExternalLinkAttribs() {
 1423+ $attribs = array();
 1424+ global $wgNoFollowLinks, $wgNoFollowNsExceptions;
 1425+ $ns = $this->mTitle->getNamespace();
 1426+ if( $wgNoFollowLinks && !in_array($ns, $wgNoFollowNsExceptions) ) {
 1427+ $attribs['rel'] = 'nofollow';
 1428+ }
 1429+ if ( $this->mOptions->getExternalLinkTarget() ) {
 1430+ $attribs['target'] = $this->mOptions->getExternalLinkTarget();
 1431+ }
 1432+ return $attribs;
 1433+ }
 1434+
 1435+
14141436 /**
14151437 * Replace unusual URL escape codes with their equivalent characters
14161438 * @param string
Index: trunk/phase3/includes/Linker.php
——@@ -990,11 +990,10 @@
991991 }
992992
993993 /** @todo document */
994 - function makeExternalLink( $url, $text, $escape = true, $linktype = '', $ns = null ) {
995 - $style = $this->getExternalLinkAttributes( $url, $text, 'external ' . $linktype );
996 - global $wgNoFollowLinks, $wgNoFollowNsExceptions;
997 - if( $wgNoFollowLinks && !(isset($ns) && in_array($ns, $wgNoFollowNsExceptions)) ) {
998 - $style .= ' rel="nofollow"';
 994+ function makeExternalLink( $url, $text, $escape = true, $linktype = '', $attribs = array() ) {
 995+ $attribsText = $this->getExternalLinkAttributes( $url, $text, 'external ' . $linktype );
 996+ if ( $attribs ) {
 997+ $attribsText .= ' ' . Xml::expandAttributes( $attribs );
999998 }
1000999 $url = htmlspecialchars( $url );
10011000 if( $escape ) {
——@@ -1006,7 +1005,7 @@
10071006 wfDebug("Hook LinkerMakeExternalLink changed the output of link with url {$url} and text {$text} to {$link}", true);
10081007 return $link;
10091008 }
1010 - return '<a href="'.$url.'"'.$style.'>'.$text.'</a>';
 1009+ return '<a href="'.$url.'"'.$attribsText.'>'.$text.'</a>';
10111010 }
10121011
10131012 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r41406Back out r41333 -- causes lots of parser test regressions due to funny spacin...brion00:14, 30 September 2008
r41409Revert revert r41406 of r41333, and removed one space between attributes.tstarling01:00, 30 September 2008

Comments

#Comment by Brion VIBBER (talk | contribs)   20:38, 30 September 2008

Backed out in r41406:

Back out r41333 -- causes lots of parser test regressions due to funny spacing. Probably an easy fix but it wasn't tested apparently.  :)

#Comment by Tim Starling (talk | contribs)   01:04, 1 October 2008

Fixed in r41409

Status & tagging log