LinkRenderer: Re-implement noclasses as makePreloadedLink function
authorKunal Mehta <legoktm@member.fsf.org>
Wed, 25 May 2016 23:37:21 +0000 (16:37 -0700)
committerKunal Mehta <legoktm@member.fsf.org>
Thu, 26 May 2016 07:24:06 +0000 (00:24 -0700)
'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
includes/linker/LinkRenderer.php
includes/linker/LinkRendererFactory.php
tests/parser/parserTests.txt
tests/phpunit/includes/LinkerTest.php
tests/phpunit/includes/linker/LinkRendererFactoryTest.php

index 0b80c08..c0d9358 100644 (file)
@@ -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 );
                }
index cb3acce..1237907 100644 (file)
@@ -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!
index 3a30772..7124be1 100644 (file)
@@ -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 );
                }
index 910e9ae..c9e7887 100644 (file)
@@ -13953,7 +13953,7 @@ Redirected image
 !! wikitext
 [[Image:Barfoo.jpg]]
 !! html/php
-<p><a href="/wiki/File:Barfoo.jpg" title="File:Barfoo.jpg" class="mw-redirect">File:Barfoo.jpg</a>
+<p><a href="/wiki/File:Barfoo.jpg" class="mw-redirect" title="File:Barfoo.jpg">File:Barfoo.jpg</a>
 </p>
 !! end
 
index 1bf8729..6215ba1 100644 (file)
@@ -34,35 +34,35 @@ class LinkerTest extends MediaWikiLangTestCase {
                        # ## ANONYMOUS USER ########################################
                        [
                                '<a href="/wiki/Special:Contributions/JohnDoe" '
-                                       . 'title="Special:Contributions/JohnDoe" '
-                                       . 'class="mw-userlink mw-anonuserlink">JohnDoe</a>',
+                                       . 'class="mw-userlink mw-anonuserlink" '
+                                       . 'title="Special:Contributions/JohnDoe">JohnDoe</a>',
                                0, 'JohnDoe', false,
                        ],
                        [
                                '<a href="/wiki/Special:Contributions/::1" '
-                                       . 'title="Special:Contributions/::1" '
-                                       . 'class="mw-userlink mw-anonuserlink">::1</a>',
+                                       . 'class="mw-userlink mw-anonuserlink" '
+                                       . 'title="Special:Contributions/::1">::1</a>',
                                0, '::1', false,
                                'Anonymous with pretty IPv6'
                        ],
                        [
                                '<a href="/wiki/Special:Contributions/0:0:0:0:0:0:0:1" '
-                                       . 'title="Special:Contributions/0:0:0:0:0:0:0:1" '
-                                       . 'class="mw-userlink mw-anonuserlink">::1</a>',
+                                       . 'class="mw-userlink mw-anonuserlink" '
+                                       . 'title="Special:Contributions/0:0:0:0:0:0:0:1">::1</a>',
                                0, '0:0:0:0:0:0:0:1', false,
                                'Anonymous with almost pretty IPv6'
                        ],
                        [
                                '<a href="/wiki/Special:Contributions/0000:0000:0000:0000:0000:0000:0000:0001" '
-                                       . 'title="Special:Contributions/0000:0000:0000:0000:0000:0000:0000:0001" '
-                                       . 'class="mw-userlink mw-anonuserlink">::1</a>',
+                                       . 'class="mw-userlink mw-anonuserlink" '
+                                       . 'title="Special:Contributions/0000:0000:0000:0000:0000:0000:0000:0001">::1</a>',
                                0, '0000:0000:0000:0000:0000:0000:0000:0001', false,
                                'Anonymous with full IPv6'
                        ],
                        [
                                '<a href="/wiki/Special:Contributions/::1" '
-                                       . 'title="Special:Contributions/::1" '
-                                       . 'class="mw-userlink mw-anonuserlink">AlternativeUsername</a>',
+                                       . 'class="mw-userlink mw-anonuserlink" '
+                                       . 'title="Special:Contributions/::1">AlternativeUsername</a>',
                                0, '::1', 'AlternativeUsername',
                                'Anonymous with pretty IPv6 and an alternative username'
                        ],
@@ -70,15 +70,15 @@ class LinkerTest extends MediaWikiLangTestCase {
                        # IPV4
                        [
                                '<a href="/wiki/Special:Contributions/127.0.0.1" '
-                                       . 'title="Special:Contributions/127.0.0.1" '
-                                       . 'class="mw-userlink mw-anonuserlink">127.0.0.1</a>',
+                                       . 'class="mw-userlink mw-anonuserlink" '
+                                       . 'title="Special:Contributions/127.0.0.1">127.0.0.1</a>',
                                0, '127.0.0.1', false,
                                'Anonymous with IPv4'
                        ],
                        [
                                '<a href="/wiki/Special:Contributions/127.0.0.1" '
-                                       . 'title="Special:Contributions/127.0.0.1" '
-                                       . 'class="mw-userlink mw-anonuserlink">AlternativeUsername</a>',
+                                       . 'class="mw-userlink mw-anonuserlink" '
+                                       . 'title="Special:Contributions/127.0.0.1">AlternativeUsername</a>',
                                0, '127.0.0.1', 'AlternativeUsername',
                                'Anonymous with IPv4 and an alternative username'
                        ],
index bd3103b..20e0108 100644 (file)
@@ -21,11 +21,6 @@ class LinkRendererFactoryTest extends MediaWikiLangTestCase {
 
        public static function provideCreateFromLegacyOptions() {
                return [
-                       [
-                               [ 'noclasses' ],
-                               'getNoClasses',
-                               true
-                       ],
                        [
                                [ 'forcearticlepath' ],
                                'getForceArticlePath',