* Output title before class in Linker::link() to match behavior of makeLink() and...

* Do not add action=edit to nonexistent special pages.
* Add profiling point for the bit where we add classes in linkAttribs().
* Turn makeLinkObj(), makeKnownLinkObj(), makeBrokenLinkObj() into wrappers for link().  This requires the creation of two new functions to turn query strings/attribute strings into arrays, but still results in fewer LOC (-11 lines) due to less code duplication.  This should be well-tested by the parser tests, because pretty much all link creation now goes through link(), but the only changes are encoding single quotes in attributes, which is a good change.  I find no additional database queries, so since this isn't a CPU bottleneck, there should be no performance issues.

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index c0d7fe9..c5d5f1f 100644 (file)
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -1055,6 +1055,34 @@ function wfArrayToCGI( $array1, $array2 = NULL )
        return $cgi;
 }
 
+/**
+ * This is the logical opposite of wfArrayToCGI(): it accepts a query string as
+ * its argument and returns the same string in array form.  This allows compa-
+ * tibility with legacy functions that accept raw query strings instead of nice
+ * arrays.  Of course, keys and values are urldecode()d.  Don't try passing in-
+ * valid query strings, or it will explode.
+ *
+ * @param $query string Query string
+ * @return array Array version of input
+ */
+function wfCgiToArray( $query ) {
+       if( isset( $query[0] ) and $query[0] == '?' ) {
+               $query = substr( $query, 1 );
+       }
+       $bits = explode( '&', $query );
+       $ret = array();
+       foreach( $bits as $bit ) {
+               if( $bit === '' ) {
+                       continue;
+               }
+               list( $key, $value ) = explode( '=', $bit );
+               $key = urldecode( $key );
+               $value = urldecode( $value );
+               $ret[$key] = $value;
+       }
+       return $ret;
+}
+
 /**
  * Append a query string to an existing URL, which may or may not already
  * have query string parameters already. If so, they will be combined.

diff --git a/includes/Linker.php b/includes/Linker.php
index cfd7342..c80c286 100644 (file)
--- a/includes/Linker.php
+++ b/includes/Linker.php
@@ -202,7 +202,7 @@ class Linker {
                        $this->linkAttribs( $target, $customAttribs, $options )
                );
                if( is_null( $text ) ) {
-                       $text = $this->linkText( $target, $options );
+                       $text = $this->linkText( $target );
                }
 
                $ret = Xml::openElement( 'a', $attribs )
@@ -216,8 +216,10 @@ class Linker {
        private function linkUrl( $target, $query, $options ) {
                wfProfileIn( __METHOD__ );
                # If it's a broken link, add the appropriate query pieces, unless
-               # there's already an action specified.
-               if( in_array( 'broken', $options ) and empty( $query['action'] ) ) {
+               # there's already an action specified, or unless 'edit' makes no sense
+               # (i.e., for a nonexistent special page).
+               if( in_array( 'broken', $options ) and empty( $query['action'] )
+               and $target->getNamespace() != NS_SPECIAL ) {
                        $query['action'] = 'edit';
                        $query['redlink'] = '1';
                }
@@ -231,14 +233,8 @@ class Linker {
                global $wgUser;
                $defaults = array();
 
-               # First get a default title attribute.
-               if( in_array( 'known', $options ) ) {
-                       $defaults['title'] = $target->getPrefixedText();
-               } else {
-                       $defaults['title'] = wfMsg( 'red-link-title', $target->getPrefixedText() );
-               }
-
                if( !in_array( 'noclasses', $options ) ) {
+                       wfProfileIn( __METHOD__ . '-getClasses' );
                        # Now build the classes.
                        $classes = array();
 
@@ -263,6 +259,14 @@ class Linker {
                        if( $classes != array() ) {
                                $defaults['class'] = implode( ' ', $classes );
                        }
+                       wfProfileOut( __METHOD__ . '-getClasses' );
+               }
+
+               # Get a default title attribute.
+               if( in_array( 'known', $options ) ) {
+                       $defaults['title'] = $target->getPrefixedText();
+               } else {
+                       $defaults['title'] = wfMsg( 'red-link-title', $target->getPrefixedText() );
                }
 
                # Finally, merge the custom attribs with the default ones, and iterate
@@ -280,7 +284,7 @@ class Linker {
                return $ret;
        }
 
-       private function linkText( $target, $options ) {
+       private function linkText( $target ) {
                # If the target is just a fragment, with no title, we return the frag-
                # ment text.  Otherwise, we return the title text itself.
                if( $target->getPrefixedText() === '' and $target->getFragment() !== '' ) {
@@ -397,58 +401,17 @@ class Linker {
                global $wgUser;
                wfProfileIn( __METHOD__ );
 
-               if ( $nt->isExternal() ) {
-                       $u = $nt->getFullURL();
-                       $link = $nt->getPrefixedURL();
-                       if ( '' == $text ) { $text = $nt->getPrefixedText(); }
-                       $style = $this->getInterwikiLinkAttributes( $link, $text, 'extiw' );
-
-                       $inside = '';
-                       if ( '' != $trail ) {
-                               $m = array();
-                               if ( preg_match( '/^([a-z]+)(.*)$$/sD', $trail, $m ) ) {
-                                       $inside = $m[1];
-                                       $trail = $m[2];
-                               }
-                       }
-                       $t = "<a href=\"{$u}\"{$style}>{$text}{$inside}</a>";
-
-                       wfProfileOut( __METHOD__ );
-                       return $t;
-               } elseif ( $nt->isAlwaysKnown() ) {
-                       # Image links, special page links and self-links with fragments are always known.
-                       $retVal = $this->makeKnownLinkObj( $nt, $text, $query, $trail, $prefix );
-               } else {
-                       wfProfileIn( __METHOD__.'-immediate' );
+               $query = wfCgiToArray( $query );
+               list( $inside, $trail ) = Linker::splitTrail( $trail );
+               if( $text === '' ) {
+                       $text = $this->linkText( $nt );
+               }
 
-                       # Handles links to special pages which do not exist in the database:
-                       if( $nt->getNamespace() == NS_SPECIAL ) {
-                               if( SpecialPage::exists( $nt->getDBkey() ) ) {
-                                       $retVal = $this->makeKnownLinkObj( $nt, $text, $query, $trail, $prefix );
-                               } else {
-                                       $retVal = $this->makeBrokenLinkObj( $nt, $text, $query, $trail, $prefix );
-                               }
-                               wfProfileOut( __METHOD__.'-immediate' );
-                               wfProfileOut( __METHOD__ );
-                               return $retVal;
-                       }
+               $ret = $this->link( $nt, "$prefix$text$inside", array(), $query,
+                       'noclasses' ) . $trail;
 
-                       # Work out link colour immediately
-                       $aid = $nt->getArticleID() ;
-                       if ( 0 == $aid ) {
-                               $retVal = $this->makeBrokenLinkObj( $nt, $text, $query, $trail, $prefix );
-                       } else {
-                               $colour = '';
-                               if ( $nt->isContentPage() ) {
-                                       $threshold = $wgUser->getOption('stubthreshold');
-                                       $colour = $this->getLinkColour( $nt, $threshold );
-                               }
-                               $retVal = $this->makeColouredLinkObj( $nt, $colour, $text, $query, $trail, $prefix );
-                       }
-                       wfProfileOut( __METHOD__.'-immediate' );
-               }
                wfProfileOut( __METHOD__ );
-               return $retVal;
+               return $ret;
        }
 
        /**
@@ -468,31 +431,21 @@ class Linker {
        function makeKnownLinkObj( Title $title, $text = '', $query = '', $trail = '', $prefix = '' , $aprops = '', $style = '' ) {
                wfProfileIn( __METHOD__ );
 
-               $nt = $this->normaliseSpecialPage( $title );
-
-               $u = $nt->escapeLocalURL( $query );
-               if ( $nt->getFragment() != '' ) {
-                       if( $nt->getPrefixedDbkey() == '' ) {
-                               $u = '';
-                               if ( '' == $text ) {
-                                       $text = htmlspecialchars( $nt->getFragment() );
-                               }
-                       }
-                       $u .= $nt->getFragmentForURL();
-               }
                if ( $text == '' ) {
-                       $text = htmlspecialchars( $nt->getPrefixedText() );
-               }
-               if ( $style == '' ) {
-                       $style = $this->getInternalLinkAttributesObj( $nt, $text );
+                       $text = $this->linkText( $title );
                }
+               $attribs = Sanitizer::mergeAttributes(
+                       Xml::explodeAttributes( $aprops ),
+                       Xml::explodeAttributes( $style )
+               );
+               $query = wfCgiToArray( $query );
+               list( $inside, $trail ) = Linker::splitTrail( $trail );
 
-               if ( $aprops !== '' ) $aprops = " $aprops";
+               $ret = $this->link( $title, "$prefix$text$inside", $attribs, $query,
+                       array( 'known', 'noclasses' ) ) . $trail;
 
-               list( $inside, $trail ) = Linker::splitTrail( $trail );
-               $r = "<a href=\"{$u}\"{$style}{$aprops}>{$prefix}{$text}{$inside}</a>{$trail}";
                wfProfileOut( __METHOD__ );
-               return $r;
+               return $ret;
        }
 
        /**
@@ -508,32 +461,15 @@ class Linker {
        function makeBrokenLinkObj( Title $title, $text = '', $query = '', $trail = '', $prefix = '' ) {
                wfProfileIn( __METHOD__ );
 
-               $nt = $this->normaliseSpecialPage( $title );
-
-               if( $nt->getNamespace() == NS_SPECIAL ) {
-                       $q = $query;
-               } else if ( '' == $query ) {
-                       $q = 'action=edit&redlink=1';
-               } else {
-                       $q = 'action=edit&redlink=1&'.$query;
-               }
-               $u = $nt->escapeLocalURL( $q );
-               if( $nt->getFragmentForURL() !== '' ) {
-                       # Might seem pointless to have a fragment on a redlink, but let's
-                       # be obedient.
-                       $u .= $nt->getFragmentForURL();
-               }
-
-               $titleText = $nt->getPrefixedText();
-               if ( '' == $text ) {
-                       $text = htmlspecialchars( $titleText );
-               }
-               $titleAttr = wfMsg( 'red-link-title', $titleText );
-               $style = $this->getInternalLinkAttributesObj( $nt, $text, 'new', $titleAttr );
                list( $inside, $trail ) = Linker::splitTrail( $trail );
+               if( $text === '' ) {
+                       $text = $this->linkText( $title );
+               }
+               $nt = $this->normaliseSpecialPage( $title );
 
                wfRunHooks( 'BrokenLink', array( &$this, $nt, $query, &$u, &$style, &$prefix, &$text, &$inside, &$trail ) );
-               $s = "<a href=\"{$u}\"{$style}>{$prefix}{$text}{$inside}</a>{$trail}";
+               $s = $this->link( $title, "$prefix$text$inside", array(),
+                       wfCgiToArray( $query ), array( 'broken', 'noclasses' ) ) . $trail;
 
                wfProfileOut( __METHOD__ );
                return $s;

diff --git a/includes/Xml.php b/includes/Xml.php
index 7db9ee8..b5daf18 100644 (file)
--- a/includes/Xml.php
+++ b/includes/Xml.php
@@ -54,6 +54,31 @@ class Xml {
                }
        }
 
+       /**
+        * Given a string of attributes for an element, return an array of key =>
+        * value pairs.  Can be used for backward compatibility with old functions
+        * that accept attributes as strings instead of arrays.  Does not validate
+        * the string, so watch out for GIGO.
+        *
+        * @param $attribs string
+        * @return array
+        */
+       public static function explodeAttributes( $attribs ) {
+               $matches = array();
+               preg_match_all( '/([^\s=\'"]+)\s*=\s*(?:\'([^\']*)\'|"([^"]*)")/',
+                       $attribs, $matches );
+               $ret = array();
+               for( $i = 0; $i < count( $matches[0] ); ++$i ) {
+                       if( $matches[2][$i] !== '' ) {
+                               $val = $matches[2][$i];
+                       } else {
+                               $val = $matches[3][$i];
+                       }
+                       $ret[$matches[1][$i]] = html_entity_decode( $val );
+               }
+               return $ret;
+       }
+
        /**
         * Format an XML element as with self::element(), but run text through the
         * UtfNormal::cleanUp() validator first to ensure that no invalid UTF-8
@@ -682,4 +707,4 @@ class XmlSelect {
                return Xml::tags( 'select', $this->attributes, implode( "\n", $this->options ) );
        }
 
-}
\ No newline at end of file
+}

diff --git a/maintenance/parserTests.txt b/maintenance/parserTests.txt
index fd17ca6..9379668 100644 (file)
--- a/maintenance/parserTests.txt
+++ b/maintenance/parserTests.txt
@@ -1600,7 +1600,7 @@ Inline interwiki link
 !! input
 [[MeatBall:SoftSecurity]]
 !! result
-<p><a href="http://www.usemod.com/cgi-bin/mb.pl?SoftSecurity" title="meatball:SoftSecurity" class="extiw">MeatBall:SoftSecurity</a>
+<p><a href="http://www.usemod.com/cgi-bin/mb.pl?SoftSecurity" class="extiw" title="meatball:SoftSecurity">MeatBall:SoftSecurity</a>
 </p>
 !! end
 
@@ -1609,7 +1609,7 @@ Inline interwiki link with empty title (bug 2372)
 !! input
 [[MeatBall:]]
 !! result
-<p><a href="http://www.usemod.com/cgi-bin/mb.pl?" title="meatball:" class="extiw">MeatBall:</a>
+<p><a href="http://www.usemod.com/cgi-bin/mb.pl?" class="extiw" title="meatball:">MeatBall:</a>
 </p>
 !! end
 
@@ -1619,8 +1619,8 @@ Interwiki link encoding conversion (bug 1636)
 *[[Wikipedia:ro:Olteni&#0355;a]]
 *[[Wikipedia:ro:Olteni&#355;a]]
 !! result
-<ul><li><a href="http://en.wikipedia.org/wiki/ro:Olteni%C5%A3a" title="wikipedia:ro:Olteniţa" class="extiw">Wikipedia:ro:Olteni&#355;a</a>
-</li><li><a href="http://en.wikipedia.org/wiki/ro:Olteni%C5%A3a" title="wikipedia:ro:Olteniţa" class="extiw">Wikipedia:ro:Olteni&#355;a</a>
+<ul><li><a href="http://en.wikipedia.org/wiki/ro:Olteni%C5%A3a" class="extiw" title="wikipedia:ro:Olteniţa">Wikipedia:ro:Olteni&#355;a</a>
+</li><li><a href="http://en.wikipedia.org/wiki/ro:Olteni%C5%A3a" class="extiw" title="wikipedia:ro:Olteniţa">Wikipedia:ro:Olteni&#355;a</a>
 </li></ul>
 
 !! end
@@ -1630,7 +1630,7 @@ Interwiki link with fragment (bug 2130)
 !! input
 [[MeatBall:SoftSecurity#foo]]
 !! result
-<p><a href="http://www.usemod.com/cgi-bin/mb.pl?SoftSecurity#foo" title="meatball:SoftSecurity" class="extiw">MeatBall:SoftSecurity#foo</a>
+<p><a href="http://www.usemod.com/cgi-bin/mb.pl?SoftSecurity#foo" class="extiw" title="meatball:SoftSecurity">MeatBall:SoftSecurity#foo</a>
 </p>
 !! end
 
@@ -3339,7 +3339,7 @@ Link to category
 !! input
 [[:Category:MediaWiki User's Guide]]
 !! result
-<p><a href="/wiki/Category:MediaWiki_User%27s_Guide" title="Category:MediaWiki User's Guide">Category:MediaWiki User's Guide</a>
+<p><a href="/wiki/Category:MediaWiki_User%27s_Guide" title="Category:MediaWiki User&#39;s Guide">Category:MediaWiki User's Guide</a>
 </p>
 !! end
 
@@ -3350,7 +3350,7 @@ cat
 !! input
 [[Category:MediaWiki User's Guide]]
 !! result
-<a href="/wiki/Category:MediaWiki_User%27s_Guide" title="Category:MediaWiki User's Guide">MediaWiki User's Guide</a>
+<a href="/wiki/Category:MediaWiki_User%27s_Guide" title="Category:MediaWiki User&#039;s Guide">MediaWiki User's Guide</a>
 !! end
 
 !! test
@@ -6625,7 +6625,7 @@ language=sr cat
 !! input
 [[:Category:МедиаWики Усер'с Гуиде]]
 !! result
-<a href="/wiki/Category:MediaWiki_User%27s_Guide" title="Category:MediaWiki User's Guide">MediaWiki User's Guide</a>
+<a href="/wiki/Category:MediaWiki_User%27s_Guide" title="Category:MediaWiki User&#039;s Guide">MediaWiki User's Guide</a>
 !! end