jenkins-bot has submitted this change and it was merged.
Change subject: Resolve language confusion in terms view
......................................................................
Resolve language confusion in terms view
There is quite some confusion going on here.
Note that this class does have 3 public methods.
When requesting HTML via getHtml() (this is for the label,
description and aliases on top of the page) and getTitleHtml() the
$this->languageCode is *not* the interface language but the language
of the requested term. Messages use wfMessage(), which defaults to
$wgLang, which is set to qqx in the test.
But when calling getEntityTermsForLanguageListView() the same
$this->languageCode property is misused as the interface language.
Luckily this is not an issue in production because
$this->languageCode is always set to the users interface language.
This also splits a test with a bad data provider (note the removed
$numberOfPlaceholders and substr_count) and adds a bunch of missing
tests and assertions.
Change-Id: I4270a15e8f3e516d154d1a4d1018a9760c5ba95d
---
M view/src/EntityTermsView.php
M view/tests/phpunit/EntityTermsViewTest.php
2 files changed, 58 insertions(+), 34 deletions(-)
Approvals:
Adrian Lang: Looks good to me, approved
jenkins-bot: Verified
diff --git a/view/src/EntityTermsView.php b/view/src/EntityTermsView.php
index a23ed17..a27f852 100644
--- a/view/src/EntityTermsView.php
+++ b/view/src/EntityTermsView.php
@@ -35,7 +35,7 @@
private $sectionEditLinkGenerator;
/**
- * @var string
+ * @var string Language of the terms in the title and header section.
*/
private $languageCode;
@@ -48,7 +48,8 @@
* @param TemplateFactory $templateFactory
* @param EditSectionGenerator|null $sectionEditLinkGenerator
* @param LanguageNameLookup $languageNameLookup
- * @param string $languageCode
+ * @param string $languageCode Desired language of the label,
description and aliases in the
+ * title and header section. Not necessarily identical to the
interface language.
*/
public function __construct(
TemplateFactory $templateFactory,
@@ -207,7 +208,9 @@
$languageCode,
Title $title = null
) {
- $languageName = $this->languageNameLookup->getName(
$languageCode, $this->languageCode );
+ global $wgLang;
+
+ $languageName = $this->languageNameLookup->getName(
$languageCode, $wgLang->getCode() );
$labels = $fingerprint->getLabels();
$descriptions = $fingerprint->getDescriptions();
$hasLabel = $labels->hasTermForLanguage( $languageCode );
@@ -299,7 +302,7 @@
* @return Message
*/
private function msg( $key ) {
- return wfMessage( $key )->inLanguage( $this->languageCode );
+ return wfMessage( $key );
}
}
diff --git a/view/tests/phpunit/EntityTermsViewTest.php
b/view/tests/phpunit/EntityTermsViewTest.php
index 9f7e7d3..6175784 100644
--- a/view/tests/phpunit/EntityTermsViewTest.php
+++ b/view/tests/phpunit/EntityTermsViewTest.php
@@ -49,7 +49,9 @@
$languageNameLookup = $this->getMock(
'Wikibase\Lib\LanguageNameLookup' );
$languageNameLookup->expects( $this->exactly(
$languageNameCalls ) )
->method( 'getName' )
- ->will( $this->returnValue( '<LANGUAGENAME>' ) );
+ ->will( $this->returnCallback( function( $languageCode,
$inLanguage = null ) {
+ return
"<LANGUAGENAME-$languageCode-IN-$inLanguage>";
+ } ) );
return new EntityTermsView(
TemplateFactory::getDefaultInstance(),
@@ -110,30 +112,16 @@
$this->assertNotContains( '<script>', $html );
$this->assertNotContains( '<b>', $html );
$this->assertNotContains( '<i>', $html );
+ $this->assertNotContains( '&', $html, 'no double escaping'
);
}
- public function emptyFingerprintProvider() {
- $noDescription = $this->getFingerprint();
- $noDescription->removeDescription( 'en' );
-
- $noAliases = $this->getFingerprint();
- $noAliases->removeAliasGroup( 'en' );
-
- return array(
- array( new Fingerprint(), '-empty)' ),
- array( $noDescription, '(wikibase-description-empty)' ),
- array( $noAliases, '(wikibase-aliases-empty)' ),
- );
- }
-
- /**
- * @dataProvider emptyFingerprintProvider
- */
- public function testGetHtml_isMarkedAsEmptyValue( Fingerprint
$fingerprint, $expectedPlaceholder ) {
+ public function testGetHtml_isMarkedAsEmptyValue() {
$entityTermsView = $this->getEntityTermsView( 1 );
- $html = $entityTermsView->getHtml( $fingerprint, null, '', new
TextInjector() );
+ $html = $entityTermsView->getHtml( new Fingerprint(), null, '',
new TextInjector() );
$this->assertContains( 'wb-empty', $html );
+ $this->assertContains( '(wikibase-description-empty)', $html );
+ $this->assertContains( '(wikibase-aliases-empty)', $html );
}
public function testGetHtml_isNotMarkedAsEmpty() {
@@ -141,18 +129,32 @@
$html = $entityTermsView->getHtml( $this->getFingerprint(),
null, '', new TextInjector() );
$this->assertNotContains( 'wb-empty', $html );
+ $this->assertNotContains( '(wikibase-description-empty)', $html
);
+ $this->assertNotContains( '(wikibase-aliases-empty)', $html );
}
- /**
- * @dataProvider emptyFingerprintProvider
- */
- public function testGetHtml_containsIsEmptyPlaceholders( Fingerprint
$fingerprint, $expectedPlaceholder ) {
- $entityTermsView = $this->getEntityTermsView( 1 );
- $html = $entityTermsView->getHtml( $fingerprint, null, '', new
TextInjector() );
+ public function testGetHtml_containsEmptyDescriptionPlaceholder() {
+ $fingerprint = $this->getFingerprint();
+ $fingerprint->removeDescription( 'en' );
- $this->assertContains( $expectedPlaceholder, $html );
- $numberOfPlaceholders = $fingerprint->isEmpty() ? 2 : 1;
- $this->assertSame( $numberOfPlaceholders, substr_count( $html,
$expectedPlaceholder ) );
+ $view = $this->getEntityTermsView( 1 );
+ $html = $view->getHtml( $fingerprint, null, '', new
TextInjector() );
+
+ $this->assertContains( 'wb-empty', $html );
+ $this->assertContains( '(wikibase-description-empty)', $html );
+ $this->assertNotContains( '(wikibase-aliases-empty)', $html );
+ }
+
+ public function testGetHtml_containsEmptyAliasesPlaceholder() {
+ $fingerprint = $this->getFingerprint();
+ $fingerprint->removeAliasGroup( 'en' );
+
+ $view = $this->getEntityTermsView( 1 );
+ $html = $view->getHtml( $fingerprint, null, '', new
TextInjector() );
+
+ $this->assertContains( 'wb-empty', $html );
+ $this->assertNotContains( '(wikibase-description-empty)', $html
);
+ $this->assertContains( '(wikibase-aliases-empty)', $html );
}
public function testGetTitleHtml_containsLabel() {
@@ -189,6 +191,7 @@
$this->assertContains( 'evil html', $html, 'make sure it works'
);
$this->assertNotContains( 'href="#"', $html );
+ $this->assertNotContains( '&', $html, 'no double escaping'
);
}
public function testGetTitleHtml_isMarkedAsEmpty() {
@@ -199,6 +202,7 @@
$html = $entityTermsView->getTitleHtml( $fingerprint, null );
$this->assertContains( 'wb-empty', $html );
+ $this->assertContains( '(wikibase-label-empty)', $html );
}
public function testGetTitleHtml_isNotMarkedAsEmpty() {
@@ -208,6 +212,7 @@
$html = $entityTermsView->getTitleHtml( $fingerprint, null );
$this->assertNotContains( 'wb-empty', $html );
+ $this->assertNotContains( '(wikibase-label-empty)', $html );
}
public function testGetEntityTermsForLanguageListView() {
@@ -220,13 +225,29 @@
$view = $this->getEntityTermsView( 0, 1 );
$html = $view->getEntityTermsForLanguageListView( $fingerprint,
array( 'en' ), $title );
+ $this->assertContains(
'(wikibase-entitytermsforlanguagelistview-language)', $html );
+ $this->assertContains(
'(wikibase-entitytermsforlanguagelistview-label)', $html );
+ $this->assertContains(
'(wikibase-entitytermsforlanguagelistview-description)', $html );
+ $this->assertContains(
'(wikibase-entitytermsforlanguagelistview-aliases)', $html );
+
$this->assertContains(
'wikibase-entitytermsforlanguageview-en', $html );
$this->assertContains( '<LOCALURL>', $html );
- $this->assertContains( '<LANGUAGENAME>', $html );
+ $this->assertContains( '<LANGUAGENAME-en-IN-qqx>', $html
);
$this->assertContains( '<LABEL>', $html );
$this->assertContains( '<DESCRIPTION>', $html );
$this->assertContains( '<ALIAS1>', $html );
$this->assertContains( '<ALIAS2>', $html );
+ $this->assertNotContains( '&', $html, 'no double escaping'
);
+ }
+
+ public function testGetEntityTermsForLanguageListView_isMarkedAsEmpty()
{
+ $view = $this->getEntityTermsView( 0, 1 );
+ $html = $view->getEntityTermsForLanguageListView( new
Fingerprint(), array( 'en' ), null );
+
+ $this->assertContains( 'wb-empty', $html );
+ $this->assertContains( '(wikibase-label-empty)', $html );
+ $this->assertContains( '(wikibase-description-empty)', $html );
+ $this->assertNotContains( '(wikibase-aliases-empty)', $html );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/260336
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4270a15e8f3e516d154d1a4d1018a9760c5ba95d
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: Adrian Lang <adrian.he...@wikimedia.de>
Gerrit-Reviewer: Bene <benestar.wikime...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Jonas Kress (WMDE) <jonas.kr...@wikimedia.de>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
