Thiemo Mättig (WMDE) has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/340149 )
Change subject: Cleanups and fixes to both InterWikiLink…Formatters ...................................................................... Cleanups and fixes to both InterWikiLink…Formatters Bug: T57549 Change-Id: I12a408e51d4a951aa0e5511e6759ba5e2e7032bf --- M lib/includes/Formatters/InterWikiLinkHtmlFormatter.php M lib/includes/Formatters/InterWikiLinkWikitextFormatter.php M lib/includes/Formatters/WikibaseValueFormatterBuilders.php M lib/tests/phpunit/Formatters/InterWikiLinkHtmlFormatterTest.php M lib/tests/phpunit/Formatters/InterWikiLinkWikitextFormatterTest.php M lib/tests/phpunit/Formatters/WikibaseValueFormatterBuildersTest.php 6 files changed, 45 insertions(+), 41 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/49/340149/1 diff --git a/lib/includes/Formatters/InterWikiLinkHtmlFormatter.php b/lib/includes/Formatters/InterWikiLinkHtmlFormatter.php index 8cfd378..a833504 100644 --- a/lib/includes/Formatters/InterWikiLinkHtmlFormatter.php +++ b/lib/includes/Formatters/InterWikiLinkHtmlFormatter.php @@ -5,7 +5,6 @@ use DataValues\StringValue; use Html; use InvalidArgumentException; -use ValueFormatters\FormatterOptions; use ValueFormatters\ValueFormatter; /** @@ -17,17 +16,12 @@ class InterWikiLinkHtmlFormatter implements ValueFormatter { /** - * @var array HTML attributes to use on the generated <a> tags. - */ - private $attributes = [ 'class' => 'extiw' ]; - - /** * @var string */ private $baseUrl; /** - * @param string $baseUrl + * @param string $baseUrl Base URL, used to build links to the geo shape storage. */ public function __construct( $baseUrl ) { $this->baseUrl = $baseUrl; @@ -48,17 +42,19 @@ throw new InvalidArgumentException( 'Data value type mismatch. Expected a StringValue.' ); } - $attributes = array_merge( $this->attributes, array( - 'href' => $this->baseUrl . $this->getPathFromTitle( $value->getValue() ) - ) ); - - return Html::element( 'a', $attributes, $value->getValue() ); - + return Html::element( 'a', [ + 'class' => 'extiw', + 'href' => wfUrlencode( $this->baseUrl . $this->encodeSpaces( $value->getValue() ) ), + ], $value->getValue() ); } - private function getPathFromTitle( $title ) { - return urlencode( str_replace( ' ', '_', $title ) ); - + /** + * @param string $pageName + * + * @return string + */ + private function encodeSpaces( $pageName ) { + return str_replace( ' ', '_', $pageName ); } } diff --git a/lib/includes/Formatters/InterWikiLinkWikitextFormatter.php b/lib/includes/Formatters/InterWikiLinkWikitextFormatter.php index fe11bdf..a395dcc 100644 --- a/lib/includes/Formatters/InterWikiLinkWikitextFormatter.php +++ b/lib/includes/Formatters/InterWikiLinkWikitextFormatter.php @@ -4,7 +4,6 @@ use DataValues\StringValue; use InvalidArgumentException; -use ValueFormatters\FormatterOptions; use ValueFormatters\ValueFormatter; /** @@ -15,15 +14,13 @@ */ class InterWikiLinkWikitextFormatter implements ValueFormatter { - const OPTION_BASE_URL = 'baseUrl'; - /** * @var string */ private $baseUrl; /** - * @param string $baseUrl + * @param string $baseUrl Base URL, used to build links to the geo shape storage. */ public function __construct( $baseUrl ) { $this->baseUrl = $baseUrl; @@ -45,14 +42,19 @@ } return '[' . - wfUrlencode( $this->baseUrl . $this->getPathFromTitle( $value->getValue() ) ) . + wfUrlencode( $this->baseUrl . $this->encodeSpaces( $value->getValue() ) ) . ' ' . wfEscapeWikiText( $value->getValue() ) . ']'; } - private function getPathFromTitle( $title ) { - return str_replace( ' ', '_', $title ); + /** + * @param string $pageName + * + * @return string + */ + private function encodeSpaces( $pageName ) { + return str_replace( ' ', '_', $pageName ); } } diff --git a/lib/includes/Formatters/WikibaseValueFormatterBuilders.php b/lib/includes/Formatters/WikibaseValueFormatterBuilders.php index dea79cf..adb954e 100644 --- a/lib/includes/Formatters/WikibaseValueFormatterBuilders.php +++ b/lib/includes/Formatters/WikibaseValueFormatterBuilders.php @@ -56,6 +56,11 @@ private $repoItemUriParser; /** + * @var string + */ + private $geoShapeStorageFrontendUrl; + + /** * @var EntityTitleLookup|null */ private $entityTitleLookup; @@ -71,11 +76,6 @@ 'http://www.wikidata.org/entity/Q199', 'http://qudt.org/vocab/unit#Unitless', ); - - /** - * @var string - */ - private $geoShapeStorageUrl; /** * @param Language $defaultLanguage @@ -98,11 +98,12 @@ $geoShapeStorageFrontendUrl, '$geoShapeStorageFrontendUrl' ); + $this->defaultLanguage = $defaultLanguage; $this->labelDescriptionLookupFactory = $labelDescriptionLookupFactory; $this->languageNameLookup = $languageNameLookup; $this->repoItemUriParser = $repoItemUriParser; - $this->geoShapeStorageUrl = $geoShapeStorageFrontendUrl; + $this->geoShapeStorageFrontendUrl = $geoShapeStorageFrontendUrl; $this->entityTitleLookup = $entityTitleLookup; } @@ -256,9 +257,9 @@ public function newGeoShapeFormatter( $format, FormatterOptions $options ) { switch ( $this->getBaseFormat( $format ) ) { case SnakFormatter::FORMAT_HTML: - return new InterWikiLinkHtmlFormatter( $this->geoShapeStorageUrl ); + return new InterWikiLinkHtmlFormatter( $this->geoShapeStorageFrontendUrl ); case SnakFormatter::FORMAT_WIKI: - return new InterWikiLinkWikitextFormatter( $this->geoShapeStorageUrl ); + return new InterWikiLinkWikitextFormatter( $this->geoShapeStorageFrontendUrl ); default: return $this->newStringFormatter( $format, $options ); } diff --git a/lib/tests/phpunit/Formatters/InterWikiLinkHtmlFormatterTest.php b/lib/tests/phpunit/Formatters/InterWikiLinkHtmlFormatterTest.php index e84632d..a70ac47 100644 --- a/lib/tests/phpunit/Formatters/InterWikiLinkHtmlFormatterTest.php +++ b/lib/tests/phpunit/Formatters/InterWikiLinkHtmlFormatterTest.php @@ -5,12 +5,11 @@ use DataValues\NumberValue; use DataValues\StringValue; use InvalidArgumentException; -use ValueFormatters\FormatterOptions; use Wikibase\Lib\CommonsLinkFormatter; use Wikibase\Lib\Formatters\InterWikiLinkHtmlFormatter; /** - * @covers Wikibase\Lib\InterWikiLinkHtmlFormatter + * @covers Wikibase\Lib\Formatters\InterWikiLinkHtmlFormatter * * @group Wikibase * @group Database @@ -49,7 +48,6 @@ * @dataProvider linkFormatProvider */ public function testFormat( StringValue $value, $pattern ) { - $formatter = new InterWikiLinkHtmlFormatter( 'http://base.url/' ); $html = $formatter->format( $value ); @@ -64,4 +62,12 @@ $formatter->format( $value ); } + public function testBasePathContainsSpace_EncodesSpaceWhenFormats() { + $formatter = new InterWikiLinkHtmlFormatter( '//base.url/some wiki/' ); + + $html = $formatter->format( new StringValue( 'LINK' ) ); + + $this->assertSame( '<a class="extiw" href="//base.url/some+wiki/LINK">LINK</a>', $html ); + } + } diff --git a/lib/tests/phpunit/Formatters/InterWikiLinkWikitextFormatterTest.php b/lib/tests/phpunit/Formatters/InterWikiLinkWikitextFormatterTest.php index 3318618..b8ef856 100644 --- a/lib/tests/phpunit/Formatters/InterWikiLinkWikitextFormatterTest.php +++ b/lib/tests/phpunit/Formatters/InterWikiLinkWikitextFormatterTest.php @@ -3,7 +3,6 @@ namespace Wikibase\Lib\Tests\Formatters; use DataValues\StringValue; -use ValueFormatters\FormatterOptions; use Wikibase\Lib\Formatters\InterWikiLinkWikitextFormatter; /** diff --git a/lib/tests/phpunit/Formatters/WikibaseValueFormatterBuildersTest.php b/lib/tests/phpunit/Formatters/WikibaseValueFormatterBuildersTest.php index b689159..a6082ed 100644 --- a/lib/tests/phpunit/Formatters/WikibaseValueFormatterBuildersTest.php +++ b/lib/tests/phpunit/Formatters/WikibaseValueFormatterBuildersTest.php @@ -230,14 +230,14 @@ SnakFormatter::FORMAT_PLAIN, $this->newFormatterOptions(), new StringValue( 'http://acme.com/' ), - '@^http://acme\\.com/$@' + '@^http://acme\.com/$@' ), 'wikitext url' => array( 'Url', SnakFormatter::FORMAT_WIKI, $this->newFormatterOptions(), new StringValue( 'http://acme.com/' ), - '@^http://acme\\.com/$@' + '@^http://acme\.com/$@' ), 'html url' => array( 'Url', @@ -283,7 +283,7 @@ SnakFormatter::FORMAT_HTML, $this->newFormatterOptions(), new StringValue( 'Example.jpg' ), - '@^<a class="extiw" href="//commons\\.wikimedia\\.org/wiki/File:Example\\.jpg">Example\\.jpg</a>$@', + '@^<a class="extiw" href="//commons\.wikimedia\.org/wiki/File:Example\.jpg">Example\.jpg</a>$@', ), // geo-shape 'plain geo-shape' => array( @@ -298,7 +298,7 @@ SnakFormatter::FORMAT_HTML, $this->newFormatterOptions(), new StringValue( 'Data:GeoShape.map' ), - '@^<a class="extiw" href="//commons\\.wikimedia\\.org/wiki/Data%3AGeoShape\\.map">Data:GeoShape\\.map</a>$@', + '@^<a class="extiw" href="//commons\.wikimedia\.org/wiki/Data:GeoShape\.map">Data:GeoShape\.map</a>$@', ), 'wikitext geo-shape' => array( 'GeoShape', @@ -333,14 +333,14 @@ SnakFormatter::FORMAT_PLAIN, $this->newFormatterOptions( 'de' ), UnboundedQuantityValue::newFromNumber( '+123456.789' ), - '@^123\\.456,789$@' + '@^123\.456,789$@' ), 'quantity details' => array( 'Quantity', SnakFormatter::FORMAT_HTML_DIFF, $this->newFormatterOptions( 'de' ), QuantityValue::newFromNumber( '+123456.789' ), - '@^.*123\\.456,789.*$@' + '@^.*123\.456,789.*$@' ), // Time -- To view, visit https://gerrit.wikimedia.org/r/340149 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I12a408e51d4a951aa0e5511e6759ba5e2e7032bf Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits