jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/386521 )
Change subject: Strip highlighting before converting to html
......................................................................
Strip highlighting before converting to html
Result::stripHighlighting was called on two separate kinds of
highlighting, both before and after converting to HTML. Normalize so we
always stripHighlighting against the original highlight, rather than the
html version.
Bug: T178522
Change-Id: I07db7d9543e0160b56a54ea00ec64d8b498b481b
---
M includes/Search/Result.php
M tests/unit/Search/ResultTest.php
2 files changed, 49 insertions(+), 29 deletions(-)
Approvals:
Smalyshev: Looks good to me, approved
Cindy-the-browser-test-bot: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/includes/Search/Result.php b/includes/Search/Result.php
index 151f291..0ffc7a9 100644
--- a/includes/Search/Result.php
+++ b/includes/Search/Result.php
@@ -111,7 +111,7 @@
if ( isset( $highlights[ 'heading' ] ) ) {
$this->sectionSnippet = $this->escapeHighlightedText(
$highlights[ 'heading' ][ 0 ] );
- $this->sectionTitle = $this->findSectionTitle();
+ $this->sectionTitle = $this->findSectionTitle(
$highlights[ 'heading' ][ 0 ] );
}
if ( isset( $highlights[ 'category' ] ) ) {
@@ -227,9 +227,9 @@
/**
* @return Title
*/
- private function findSectionTitle() {
+ private function findSectionTitle( $highlighted ) {
return $this->getTitle()->createFragmentTarget(
Sanitizer::escapeIdForLink(
- $this->stripHighlighting( $this->sectionSnippet )
+ $this->stripHighlighting( $highlighted )
) );
}
diff --git a/tests/unit/Search/ResultTest.php b/tests/unit/Search/ResultTest.php
index 78a046f..743889d 100644
--- a/tests/unit/Search/ResultTest.php
+++ b/tests/unit/Search/ResultTest.php
@@ -3,27 +3,17 @@
namespace CirrusSearch\Search;
use CirrusSearch\CirrusTestCase;
+use CirrusSearch\Searcher;
use MediaWiki\MediaWikiServices;
/**
* @group CirrusSearch
*/
class ResultTest extends CirrusTestCase {
- public function testInterwikiResults() {
- $this->setMwGlobals( [
- 'wgCirrusSearchWikiToNameMap' => [
- 'es' => 'eswiki',
- ],
- ] );
- $config = MediaWikiServices::getInstance()
- ->getConfigFactory()
- ->makeConfig( 'CirrusSearch' );
- $elasticaResultSet = $this->getMockBuilder(
\Elastica\ResultSet::class )
- ->disableOriginalConstructor()
- ->getMock();
-
- $data = [
+ // @TODO In php 5.6 this could be a constant
+ private function exampleHit() {
+ return [
'_index' => 'eswiki_content_123456',
'_source' => [
'namespace' => NS_MAIN,
@@ -42,12 +32,32 @@
'heading' => [ '...' ],
],
];
- $elasticaResult = new \Elastica\Result( $data );
- $result = new Result(
- $elasticaResultSet,
- $elasticaResult,
- $config
+ }
+
+ public function testHighlightedSectionSnippet() {
+ $data = $this->exampleHit();
+ $data['highlight']['heading'] = [
Searcher::HIGHLIGHT_PRE_MARKER . 'stuff' . Searcher::HIGHLIGHT_POST_MARKER ];
+
+ $result = $this->mockResult( $data );
+ $this->assertEquals(
+ Searcher::HIGHLIGHT_PRE . 'stuff' .
Searcher::HIGHLIGHT_POST,
+ $result->getSectionSnippet()
);
+ $this->assertEquals(
+ 'stuff',
+ $result->getSectionTitle()->getFragment()
+ );
+ }
+
+ public function testInterwikiResults() {
+ $this->setMwGlobals( [
+ 'wgCirrusSearchWikiToNameMap' => [
+ 'es' => 'eswiki',
+ ],
+ ] );
+
+ $data = $this->exampleHit();
+ $result = $this->mockResult( $data );
$this->assertTrue( $result->getTitle()->isExternal(),
'isExternal' );
$this->assertTrue( $result->getRedirectTitle()->isExternal(),
'redirect isExternal' );
@@ -57,17 +67,27 @@
// do not match
$data['_source']['namespace'] = NS_HELP;
$data['_source']['namespace_text'] = 'Help';
- $elasticaResult = new \Elastica\Result( $data );
-
- $result = new Result(
- $elasticaResultSet,
- $elasticaResult,
- $config
- );
+ $result = $this->mockResult( $data );
$this->assertTrue( $result->getTitle()->isExternal(),
'isExternal namespace mismatch' );
$this->assertEquals( $result->getTitle()->getPrefixedText(),
'es:Help:Main Page' );
$this->assertTrue( $result->getRedirectTitle() === null,
'redirect is not built with ns mismatch' );
$this->assertTrue( $result->getSectionTitle()->isExternal(),
'section title isExternal' );
}
+
+ private function mockResult( $hit ) {
+ $config = MediaWikiServices::getInstance()
+ ->getConfigFactory()
+ ->makeConfig( 'CirrusSearch' );
+
+ $elasticaResultSet = $this->getMockBuilder(
\Elastica\ResultSet::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ return new Result(
+ $elasticaResultSet,
+ new \Elastica\Result( $hit ),
+ $config
+ );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/386521
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I07db7d9543e0160b56a54ea00ec64d8b498b481b
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <ebernhard...@wikimedia.org>
Gerrit-Reviewer: Cindy-the-browser-test-bot <bernhardsone...@gmail.com>
Gerrit-Reviewer: DCausse <dcau...@wikimedia.org>
Gerrit-Reviewer: Gehel <guillaume.leder...@wikimedia.org>
Gerrit-Reviewer: Smalyshev <smalys...@wikimedia.org>
Gerrit-Reviewer: Tjones <tjo...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
