Fix highlighting for phrase queries

I think the bug was introduced during a cleanup in Iaabc10c.
I don't think that " should be part of the legalSearchChars at query
time, it seems to break the regex.
The strategy here is to distinguish legalSearchChars used query time vs
the ones used at index time by introducing:
SearchEngine::legalSearchCharsForUpdate()

Bug: T167798
Change-Id: I61dc53665e26d3c6c48caed78dd3bbde9a33def7

diff --git a/includes/deferred/SearchUpdate.php b/includes/deferred/SearchUpdate.php
index b9a259b..c94ae2a 100644 (file)
--- a/includes/deferred/SearchUpdate.php
+++ b/includes/deferred/SearchUpdate.php
@@ -124,7 +124,7 @@ class SearchUpdate implements DeferrableUpdate {
                # Language-specific strip/conversion
                $text = $wgContLang->normalizeForSearch( $text );
                $se = $se ?: MediaWikiServices::getInstance()->newSearchEngine();
-               $lc = $se->legalSearchChars() . '&#;';
+               $lc = $se->legalSearchCharsForUpdate() . '&#;';
 
                $text = preg_replace( "/<\\/?\\s*[A-Za-z][^>]*?>/",
                        ' ', $wgContLang->lc( " " . $text . " " ) ); # Strip HTML markup
@@ -207,7 +207,7 @@ class SearchUpdate implements DeferrableUpdate {
                $ns = $this->title->getNamespace();
                $title = $this->title->getText();
 
-               $lc = $search->legalSearchChars() . '&#;';
+               $lc = $search->legalSearchCharsForUpdate() . '&#;';
                $t = $wgContLang->normalizeForSearch( $title );
                $t = preg_replace( "/[^{$lc}]+/", ' ', $t );
                $t = $wgContLang->lc( $t );

diff --git a/includes/search/SearchEngine.php b/includes/search/SearchEngine.php
index 4473bb2..9673aa3 100644 (file)
--- a/includes/search/SearchEngine.php
+++ b/includes/search/SearchEngine.php
@@ -206,7 +206,7 @@ abstract class SearchEngine {
        }
 
        /**
-        * Get chars legal for search.
+        * Get chars legal for search (at query time).
         * NOTE: usage as static is deprecated and preserved only as BC measure
         * @return string
         */
@@ -214,6 +214,16 @@ abstract class SearchEngine {
                return "A-Za-z_'.0-9\\x80-\\xFF\\-";
        }
 
+       /**
+        * Get chars legal for search (at index time).
+        *
+        * @since 1.30
+        * @return string
+        */
+       public function legalSearchCharsForUpdate() {
+               return static::legalSearchChars();
+       }
+
        /**
         * Set the maximum number of results to return
         * and how many to skip before returning the first.

diff --git a/includes/search/SearchMySQL.php b/includes/search/SearchMySQL.php
index 36cbbaa..2c7feeb 100644 (file)
--- a/includes/search/SearchMySQL.php
+++ b/includes/search/SearchMySQL.php
@@ -149,8 +149,8 @@ class SearchMySQL extends SearchDatabase {
                return $regex;
        }
 
-       public static function legalSearchChars() {
-               return "\"*" . parent::legalSearchChars();
+       public function legalSearchCharsForUpdate() {
+               return "\"*" . parent::legalSearchCharsForUpdate();
        }
 
        /**

diff --git a/includes/search/SearchOracle.php b/includes/search/SearchOracle.php
index c5a5ef1..2e6cb84 100644 (file)
--- a/includes/search/SearchOracle.php
+++ b/includes/search/SearchOracle.php
@@ -266,7 +266,7 @@ class SearchOracle extends SearchDatabase {
                        [] );
        }
 
-       public static function legalSearchChars() {
-               return "\"" . parent::legalSearchChars();
+       public function legalSearchCharsForUpdate() {
+               return "\"" . parent::legalSearchCharsForUpdate();
        }
 }

diff --git a/includes/search/SearchSqlite.php b/includes/search/SearchSqlite.php
index b40e1aa..5a8995d 100644 (file)
--- a/includes/search/SearchSqlite.php
+++ b/includes/search/SearchSqlite.php
@@ -141,8 +141,8 @@ class SearchSqlite extends SearchDatabase {
                return $regex;
        }
 
-       public static function legalSearchChars() {
-               return "\"*" . parent::legalSearchChars();
+       public function legalSearchCharsForUpdate() {
+               return "\"*" . parent::legalSearchCharsForUpdate();
        }
 
        /**

diff --git a/tests/phpunit/includes/search/SearchEngineTest.php b/tests/phpunit/includes/search/SearchEngineTest.php
index c74c893..6e00e53 100644 (file)
--- a/tests/phpunit/includes/search/SearchEngineTest.php
+++ b/tests/phpunit/includes/search/SearchEngineTest.php
@@ -124,6 +124,24 @@ class SearchEngineTest extends MediaWikiLangTestCase {
                        "Plain search failed" );
        }
 
+       public function testPhraseSearch() {
+               $res = $this->search->searchText( '"smithee is one who smiths"' );
+               $this->assertEquals(
+                       [ 'Smithee' ],
+                       $this->fetchIds( $res ),
+                       "Phrase search failed" );
+               $res = $this->search->searchText( '"smithee is one who smiths"' );
+               $match = $res->next();
+               $terms = [ 'smithee', 'is', 'one', 'who', 'smiths' ];
+               $snippet = "";
+               foreach ( $terms as $term ) {
+                       $snippet .= " <span class='searchmatch'>" . $term . "</span>";
+               }
+               $this->assertRegexp( '/' . preg_quote( $snippet, '/' ) . '/',
+                       $match->getTextSnippet( $res->termMatches() ),
+                       "Phrase search failed to highlight" );
+       }
+
        public function testTextPowerSearch() {
                $this->search->setNamespaces( [ 0, 1, 4 ] );
                $this->assertEquals(