Fix empty auto-summaries triggering a fatal error.
authordaniel <dkinzler@wikimedia.org>
Tue, 21 May 2019 10:16:22 +0000 (12:16 +0200)
committerdaniel <dkinzler@wikimedia.org>
Thu, 30 May 2019 14:34:34 +0000 (16:34 +0200)
Aka:  Streamline Linker::formatAutocomments() and add tests

This uses the "streamlining" for the code proposed by Thiemo
in I38edc1ad7720. I have squashed the two commits, so it now
has his code in Linker, but still has my tests as well as his.

Thiemo wrote on his patch:
This also changes the output in case there is no fragment to link to.
Before an empty `/* */` in a summary this would have created a link to
the page. I would like to argue this is not what a user expects.

Bug: T222628
Change-Id: I05408ede0e20dfd976f4057fc5baab461d2ef769

includes/Linker.php
tests/phpunit/includes/LinkerTest.php
tests/phpunit/includes/title/MediaWikiTitleCodecTest.php

index 3f50c97..eb7a44f 100644 (file)
@@ -1237,18 +1237,17 @@ class Linker {
                                                // that starts with "#". Before PHP 7 (and still on HHVM) substr() would
                                                // return false if the start offset is the end of the string.
                                                // On PHP 7+, it gracefully returns empty string instead.
-                                               if ( $section === false ) {
-                                                       $section = '';
-                                               }
-                                               if ( $local ) {
-                                                       $sectionTitle = new TitleValue( NS_MAIN, '', $section );
-                                               } else {
-                                                       $sectionTitle = $title->createFragmentTarget( $section );
-                                               }
-                                               if ( $sectionTitle ) {
+                                               if ( $section !== '' && $section !== false ) {
+                                                       if ( $local ) {
+                                                               $sectionTitle = new TitleValue( NS_MAIN, '', $section );
+                                                       } else {
+                                                               $sectionTitle = $title->createFragmentTarget( $section );
+                                                       }
                                                        $auto = Linker::makeCommentLink(
-                                                               $sectionTitle, $wgLang->getArrow() . $wgLang->getDirMark() . $sectionText,
-                                                               $wikiId, 'noclasses'
+                                                               $sectionTitle,
+                                                               $wgLang->getArrow() . $wgLang->getDirMark() . $sectionText,
+                                                               $wikiId,
+                                                               'noclasses'
                                                        );
                                                }
                                        }
index d3523c3..bdbbedd 100644 (file)
@@ -279,7 +279,7 @@ class LinkerTest extends MediaWikiLangTestCase {
                                "/* [[linkie?]] */",
                        ],
                        [
-                               '<span dir="auto"><span class="autocomment"><a href="/wiki/Special:BlankPage" title="Special:BlankPage">→‎</a>: </span> // Edit via via</span>',
+                               '<span dir="auto"><span class="autocomment">: </span> // Edit via via</span>',
                                // Regression test for T222857
                                "/*  */ // Edit via via",
                        ],
@@ -321,6 +321,36 @@ class LinkerTest extends MediaWikiLangTestCase {
                                "/* autocomment */",
                                null
                        ],
+                       [
+                               '',
+                               "/* */",
+                               false, true
+                       ],
+                       [
+                               '',
+                               "/* */",
+                               null
+                       ],
+                       [
+                               '<span dir="auto"><span class="autocomment">[[</span></span>',
+                               "/* [[ */",
+                               false, true
+                       ],
+                       [
+                               '<span dir="auto"><span class="autocomment">[[</span></span>',
+                               "/* [[ */",
+                               null
+                       ],
+                       [
+                               "foo <span dir=\"auto\"><span class=\"autocomment\"><a href=\"#.23\">→‎&#91;[#_\t_]]</a></span></span>",
+                               "foo /* [[#_\t_]] */",
+                               false, true
+                       ],
+                       [
+                               "foo <span dir=\"auto\"><span class=\"autocomment\"><a href=\"#_.09\">#_\t_</a></span></span>",
+                               "foo /* [[#_\t_]] */",
+                               null
+                       ],
                        [
                                '<span dir="auto"><span class="autocomment"><a href="/wiki/Special:BlankPage#autocomment" title="Special:BlankPage">→‎autocomment</a></span></span>',
                                "/* autocomment */",
index b8bf087..fdd200b 100644 (file)
@@ -336,6 +336,14 @@ class MediaWikiTitleCodecTest extends MediaWikiTestCase {
                                'X' . str_repeat( 'x', 251 ) ],
                        // Test decoding and normalization
                        [ '&quot;n&#x303;&#34;', NS_MAIN, 'en', new TitleValue( NS_MAIN, '"ñ"' ) ],
+                       [ 'X#n&#x303;', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'ñ' ) ],
+                       // target section parsing
+                       'empty fragment' => [ 'X#', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X' ) ],
+                       'double hash' => [ 'X##', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', '#' ) ],
+                       'fragment with hash' => [ 'X#z#z', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'z#z' ) ],
+                       'fragment with space' => [ 'X#z z', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'z z' ) ],
+                       'fragment with percent' => [ 'X#z%z', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'z%z' ) ],
+                       'fragment with amp' => [ 'X#z&z', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'z&z' ) ],
                ];
        }