From 91ac68478f6155b11242c5d05ad90deebc3b8bed Mon Sep 17 00:00:00 2001 From: Aryeh Gregor Date: Fri, 1 Aug 2008 15:02:46 +0000 Subject: [PATCH] Start finding more bugs in Linker::link() by having the Parser use it, and running the parser tests. * Support class="extiw" * Do not double the fragment for external links with fragments. Move the code for this into a new Title::getLinkUrl() instead of a Linker method, because it seems like a useful concept to be able to get a *usable* link to the current Title. * Fix a few parser tests that expected attributes in the opposite order. * Don't overwrite actions for broken links. * Style If you want me to stop doing this, by the way, please say so before I spend too many more hours of my life on it. --- includes/Linker.php | 40 +++++++++++++++---------------------- includes/Title.php | 30 ++++++++++++++++++++++++++++ includes/parser/Parser.php | 2 +- maintenance/parserTests.txt | 10 +++++----- 4 files changed, 52 insertions(+), 30 deletions(-) diff --git a/includes/Linker.php b/includes/Linker.php index 0bd1daae9e..8255e1a275 100644 --- a/includes/Linker.php +++ b/includes/Linker.php @@ -161,8 +161,9 @@ class Linker { * 'known': Page is known to exist, so don't check if it does. * 'broken': Page is known not to exist, so don't check if it does. * 'noclasses': Don't add any classes automatically (includes "new", - * "stub", "mw-redirect"). Only use the class attribute provided, if - * any. + * "stub", "mw-redirect", "extiw"). Only use the class attribute + * provided, if any, so you get a simple blue link with no funny i- + * cons. * @return string HTML attribute */ public function link( $target, $text = null, $customAttribs = array(), $query = array(), $options = array() ) { @@ -217,27 +218,13 @@ class Linker { } private function linkUrl( $target, $query, $options ) { - # If it's a broken link, add the appropriate query pieces. This over- - # writes the default action! - if( in_array( 'broken', $options ) ) { + # 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'] ) ) { $query['action'] = 'edit'; $query['redlink'] = '1'; } - $fragment = $target->getFragmentForURL(); - $title = $target->getPrefixedText(); - # A couple of things to be concerned about here. First of all, - # getLocalURL() ignores fragments. Second of all, if the Title is - # *only* a fragment, it returns something like "/". - if( $fragment === '' and $title !== '' ) { - return $target->getLocalURL( $query ); - } - if( $title === '' ) { - # Just a fragment. There had better be one, anyway, or this is a - # pretty silly Title. - return $fragment; - } - # Then we must have a fragment *and* some Title text. - return $target->getLocalURL( $query ) . $fragment; + return $target->getLinkUrl( $query ); } private function linkAttribs( $target, $attribs, $options ) { @@ -252,20 +239,25 @@ class Linker { } if( !in_array( 'noclasses', $options ) ) { - # Now build the classes. This is the bulk of what we're doing. + # Now build the classes. $classes = array(); if( in_array( 'broken', $options ) ) { - $classes []= 'new'; + $classes[] = 'new'; + } + + if( $target->isExternal() ) { + $classes[] = 'extiw'; } # Note that redirects never count as stubs here. if ( $target->isRedirect() ) { - $classes []= 'mw-redirect'; + $classes[] = 'mw-redirect'; } elseif( $target->isContentPage() ) { + # Check for stub. $threshold = $wgUser->getOption( 'stubthreshold' ); if( $threshold > 0 and $target->getLength() < $threshold ) { - $classes []= 'stub'; + $classes[] = 'stub'; } } if( $classes != array() ) { diff --git a/includes/Title.php b/includes/Title.php index b874b150e0..859d98f5ea 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -867,6 +867,36 @@ class Title { return $url; } + /** + * Get a URL that's the simplest URL that will be valid to link, locally, + * to the current Title. It includes the fragment, but does not include + * the server unless action=render is used (or the link is external). If + * there's a fragment but the prefixed text is empty, we just return a link + * to the fragment. + * + * @param array $query An associative array of key => value pairs for the + * query string. Keys and values will be escaped. + * @param string $variant Language variant of URL (for sr, zh..). Ignored + * for external links. Default is "false" (same variant as current page, + * for anonymous users). + * @return string the URL + */ + public function getLinkUrl( $query = array(), $variant = false ) { + if( !is_array( $query ) ) { + throw new MWException( 'Title::getLinkUrl passed a non-array for '. + '$query' ); + } + if( $this->isExternal() ) { + return $this->getFullURL( $query ); + } elseif( $this->getPrefixedText() === '' + and $this->getFragment() !== '' ) { + return $this->getFragmentForURL(); + } else { + return $this->getLocalURL( $query, $variant ) + . $this->getFragmentForURL(); + } + } + /** * Get an HTML-escaped version of the URL form, suitable for * using in a link, without a server name or fragment diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php index 6102259592..45a5689d0e 100644 --- a/includes/parser/Parser.php +++ b/includes/parser/Parser.php @@ -4291,7 +4291,7 @@ class Parser $replacePairs = array(); foreach( $this->mInterwikiLinkHolders['texts'] as $key => $link ) { $title = $this->mInterwikiLinkHolders['titles'][$key]; - $replacePairs[$key] = $sk->makeLinkObj( $title, $link ); + $replacePairs[$key] = $sk->link( $title, $link ); } $replacer = new HashtableReplacer( $replacePairs, 1 ); diff --git a/maintenance/parserTests.txt b/maintenance/parserTests.txt index a9f4e0a01a..b8ef2590fa 100644 --- a/maintenance/parserTests.txt +++ b/maintenance/parserTests.txt @@ -1573,7 +1573,7 @@ Inline interwiki link !! input [[MeatBall:SoftSecurity]] !! result -

MeatBall:SoftSecurity +

MeatBall:SoftSecurity

!! end @@ -1582,7 +1582,7 @@ Inline interwiki link with empty title (bug 2372) !! input [[MeatBall:]] !! result -

MeatBall: +

MeatBall:

!! end @@ -1592,8 +1592,8 @@ Interwiki link encoding conversion (bug 1636) *[[Wikipedia:ro:Olteniţa]] *[[Wikipedia:ro:Olteniţa]] !! result -