From a95e8e7262af964d843e7f4c7ab376e6b936b3f7 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Wed, 25 May 2016 16:37:21 -0700 Subject: [PATCH] LinkRenderer: Re-implement noclasses as makePreloadedLink function 'noclasses' makes more sense as a per-link option rather than an instance member of the LinkRenderer instance, since it depends entirely on whether the calling code has preloaded the link classes. Introduce LinkRenderer::makePreloadedLink() which makes this clear and requires passing in the classes as a separate parameter. As a side- effect, due to the way LinkRenderer::mergeAttribs() is implemented, the 'class' attribute will always appear before the 'title' attribute in the final output. Change-Id: I0545aa9d7139794bc22f9d3d6d6eccde003b2982 --- includes/Linker.php | 2 + includes/linker/LinkRenderer.php | 80 +++++++++---------- includes/linker/LinkRendererFactory.php | 4 - tests/parser/parserTests.txt | 2 +- tests/phpunit/includes/LinkerTest.php | 28 +++---- .../linker/LinkRendererFactoryTest.php | 5 -- 6 files changed, 54 insertions(+), 67 deletions(-) diff --git a/includes/Linker.php b/includes/Linker.php index 0b80c08830..c0d9358710 100644 --- a/includes/Linker.php +++ b/includes/Linker.php @@ -235,6 +235,8 @@ class Linker { return $linkRenderer->makeKnownLink( $target, $text, $customAttribs, $query ); } elseif ( in_array( 'broken', $options, true ) ) { return $linkRenderer->makeBrokenLink( $target, $text, $customAttribs, $query ); + } elseif ( in_array( 'noclasses', $options, true ) ) { + return $linkRenderer->makePreloadedLink( $target, $text, '', $customAttribs, $query ); } else { return $linkRenderer->makeLink( $target, $text, $customAttribs, $query ); } diff --git a/includes/linker/LinkRenderer.php b/includes/linker/LinkRenderer.php index cb3accebee..123790764c 100644 --- a/includes/linker/LinkRenderer.php +++ b/includes/linker/LinkRenderer.php @@ -52,13 +52,6 @@ class LinkRenderer { */ private $expandUrls = false; - /** - * Whether extra classes should be added - * - * @var bool - */ - private $noClasses = false; - /** * @var int */ @@ -111,20 +104,6 @@ class LinkRenderer { return $this->expandUrls; } - /** - * @param bool $no - */ - public function setNoClasses( $no ) { - $this->noClasses = $no; - } - - /** - * @return bool - */ - public function getNoClasses() { - return $this->noClasses; - } - /** * @param int $threshold */ @@ -172,9 +151,6 @@ class LinkRenderer { */ private function getLegacyOptions( $isKnown ) { $options = [ 'stubThreshold' => $this->stubThreshold ]; - if ( $this->noClasses ) { - $options[] = 'noclasses'; - } if ( $this->forceArticlePath ) { $options[] = 'forcearticlepath'; } @@ -249,14 +225,18 @@ class LinkRenderer { } /** + * If you have already looked up the proper CSS classes using Linker::getLinkColour() + * or some other method, use this to avoid looking it up again. + * * @param LinkTarget $target * @param string|HtmlArmor|null $text + * @param string $classes CSS classes to add * @param array $extraAttribs * @param array $query * @return string */ - public function makeKnownLink( - LinkTarget $target, $text = null, array $extraAttribs = [], array $query = [] + public function makePreloadedLink( + LinkTarget $target, $text = null, $classes, array $extraAttribs = [], array $query = [] ) { // Run begin hook $ret = $this->runBeginHook( $target, $text, $extraAttribs, $query, true ); @@ -265,22 +245,7 @@ class LinkRenderer { } $target = $this->normalizeTarget( $target ); $url = $this->getLinkURL( $target, $query ); - $attribs = []; - if ( !$this->noClasses ) { - $classes = []; - if ( $target->isExternal() ) { - $classes[] = 'extiw'; - } - $title = Title::newFromLinkTarget( $target ); - $colour = Linker::getLinkColour( $title, $this->stubThreshold ); - if ( $colour !== '' ) { - $classes[] = $colour; - } - if ( $classes ) { - $attribs['class'] = implode( ' ', $classes ); - } - } - + $attribs = [ 'class' => $classes ]; $prefixedText = $this->titleFormatter->getPrefixedText( $target ); if ( $prefixedText !== '' ) { $attribs['title'] = $prefixedText; @@ -297,6 +262,35 @@ class LinkRenderer { return $this->buildAElement( $target, $text, $attribs, true ); } + /** + * @param LinkTarget $target + * @param string|HtmlArmor|null $text + * @param array $extraAttribs + * @param array $query + * @return string + */ + public function makeKnownLink( + LinkTarget $target, $text = null, array $extraAttribs = [], array $query = [] + ) { + $classes = []; + if ( $target->isExternal() ) { + $classes[] = 'extiw'; + } + $title = Title::newFromLinkTarget( $target ); + $colour = Linker::getLinkColour( $title, $this->stubThreshold ); + if ( $colour !== '' ) { + $classes[] = $colour; + } + + return $this->makePreloadedLink( + $target, + $text, + $classes ? implode( ' ', $classes ) : '', + $extraAttribs, + $query + ); + } + /** * @param LinkTarget $target * @param string|HtmlArmor|null $text @@ -326,7 +320,7 @@ class LinkRenderer { } $url = $this->getLinkURL( $target, $query ); - $attribs = $this->noClasses ? [] : [ 'class' => 'new' ]; + $attribs = [ 'class' => 'new' ]; $prefixedText = $this->titleFormatter->getPrefixedText( $target ); if ( $prefixedText !== '' ) { // This ends up in parser cache! diff --git a/includes/linker/LinkRendererFactory.php b/includes/linker/LinkRendererFactory.php index 3a307727c8..7124be1e99 100644 --- a/includes/linker/LinkRendererFactory.php +++ b/includes/linker/LinkRendererFactory.php @@ -67,10 +67,6 @@ class LinkRendererFactory { public function createFromLegacyOptions( array $options ) { $linkRenderer = $this->create(); - if ( in_array( 'noclasses', $options, true ) ) { - $linkRenderer->setNoClasses( true ); - } - if ( in_array( 'forcearticlepath', $options, true ) ) { $linkRenderer->setForceArticlePath( true ); } diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt index 910e9ae512..c9e78876dc 100644 --- a/tests/parser/parserTests.txt +++ b/tests/parser/parserTests.txt @@ -13953,7 +13953,7 @@ Redirected image !! wikitext [[Image:Barfoo.jpg]] !! html/php -

File:Barfoo.jpg +

File:Barfoo.jpg

!! end diff --git a/tests/phpunit/includes/LinkerTest.php b/tests/phpunit/includes/LinkerTest.php index 1bf8729058..6215ba1a10 100644 --- a/tests/phpunit/includes/LinkerTest.php +++ b/tests/phpunit/includes/LinkerTest.php @@ -34,35 +34,35 @@ class LinkerTest extends MediaWikiLangTestCase { # ## ANONYMOUS USER ######################################## [ 'JohnDoe', + . 'class="mw-userlink mw-anonuserlink" ' + . 'title="Special:Contributions/JohnDoe">JohnDoe', 0, 'JohnDoe', false, ], [ '::1', + . 'class="mw-userlink mw-anonuserlink" ' + . 'title="Special:Contributions/::1">::1', 0, '::1', false, 'Anonymous with pretty IPv6' ], [ '::1', + . 'class="mw-userlink mw-anonuserlink" ' + . 'title="Special:Contributions/0:0:0:0:0:0:0:1">::1', 0, '0:0:0:0:0:0:0:1', false, 'Anonymous with almost pretty IPv6' ], [ '::1', + . 'class="mw-userlink mw-anonuserlink" ' + . 'title="Special:Contributions/0000:0000:0000:0000:0000:0000:0000:0001">::1', 0, '0000:0000:0000:0000:0000:0000:0000:0001', false, 'Anonymous with full IPv6' ], [ 'AlternativeUsername', + . 'class="mw-userlink mw-anonuserlink" ' + . 'title="Special:Contributions/::1">AlternativeUsername', 0, '::1', 'AlternativeUsername', 'Anonymous with pretty IPv6 and an alternative username' ], @@ -70,15 +70,15 @@ class LinkerTest extends MediaWikiLangTestCase { # IPV4 [ '127.0.0.1', + . 'class="mw-userlink mw-anonuserlink" ' + . 'title="Special:Contributions/127.0.0.1">127.0.0.1', 0, '127.0.0.1', false, 'Anonymous with IPv4' ], [ 'AlternativeUsername', + . 'class="mw-userlink mw-anonuserlink" ' + . 'title="Special:Contributions/127.0.0.1">AlternativeUsername', 0, '127.0.0.1', 'AlternativeUsername', 'Anonymous with IPv4 and an alternative username' ], diff --git a/tests/phpunit/includes/linker/LinkRendererFactoryTest.php b/tests/phpunit/includes/linker/LinkRendererFactoryTest.php index bd3103b41a..20e010812e 100644 --- a/tests/phpunit/includes/linker/LinkRendererFactoryTest.php +++ b/tests/phpunit/includes/linker/LinkRendererFactoryTest.php @@ -21,11 +21,6 @@ class LinkRendererFactoryTest extends MediaWikiLangTestCase { public static function provideCreateFromLegacyOptions() { return [ - [ - [ 'noclasses' ], - 'getNoClasses', - true - ], [ [ 'forcearticlepath' ], 'getForceArticlePath', -- 2.20.1