Skip to content

Commit

Permalink
Merge pull request #92 from wikimedia/tspans-everywhere
Browse files Browse the repository at this point in the history 
Make even text outside of tspans translateable
  • Loading branch information
@samwilson
samwilson authored Mar 25, 2019
2 parents 30f4b55 + 493be87 commit cada922
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 97 deletions.
42 changes: 29 additions & 13 deletions src/Model/Svg/SvgFile.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,28 @@ protected function makeTranslationReady(): bool
}
$translatableNodes[] = $tspan;
}

/** @var DOMElement $text */
foreach ($texts as $text) {
// For uniformity, if a <text> doesn't have <tspan>s, create one
if (0 === $text->getElementsByTagName('tspan')->length) {
$tspan = $this->document->createElement('tspan');
while ($text->childNodes->length > 0) {
$tspan->appendChild($text->childNodes[0]);
// Everything in a <text> should be a <tspan>s, otherwise we can't translate it
for ($i = 0; $i < count($text->childNodes); $i++) {
$node = $text->childNodes[$i];
if ('tspan' === $node->nodeName) {
continue;
}
if (XML_TEXT_NODE !== $node->nodeType) {
// Anything but tspans and text nodes is unexpected
return false;
}
if ('' === trim($node->nodeValue)) {
// Don't bother with whitespace-only nodes
continue;
}
// Wrap text in <tspan>
$tspan = $this->document->createElement('tspan');
$text->replaceChild($tspan, $node);
$tspan->appendChild($node);
$translatableNodes[] = $tspan;
$text->appendChild($tspan);
}
$translatableNodes[] = $text;
}
Expand Down Expand Up @@ -411,8 +423,14 @@ protected function analyse(): void
}

/**
* Try to return $this->inFileTranslations. If it is not cached, analyse the SVG
* and hence generate it.
* Returns a list of translations present in the loaded file, in the following format:
*
* 'message id' => [
* 'language code' => [
* 'text' => 'Translatable message',
* 'id' => 'foo',
* ...
* (other <text> or <tspan> attributes)
*
* @return mixed[]
*/
Expand Down Expand Up @@ -458,8 +476,7 @@ public function setTranslations(string $lang, array $translations): array
}

/**
* Try to return $this->savedLanguages (a list of languages which have one or more
* translations in-file). If it is not cached, analyse the SVG and hence generate it.
* Return a list of languages which have one or more translations in-file.
*
* @return string[]
*/
Expand Down Expand Up @@ -501,10 +518,9 @@ public function getSavedLanguagesFiltered(): array
}

/**
* Try to return $this->filteredTextNodes (an array of <text> nodes that contain only
* child elements). If it is not cached, analyse the SVG and hence generate it.
* Returns an array of <text> nodes that contain only child elements.
*
* @return mixed[]
* @return mixed[] The same message ID => language code => attributes mapping as in getInFileTranslations()
*/
public function getFilteredTextNodes(): array
{
Expand Down
167 changes: 87 additions & 80 deletions tests/Model/Svg/SvgFileTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,15 @@ class SvgFileTest extends TestCase
[
'fr' =>
[
'text' => 'et toi',
'text' => 'et toi?',
'x' => '117.42857',
'y' => '368.64789',
'id' => 'tspan2999-fr',
'data-parent' => 'text2995',
],
'nl' =>
[
'text' => 'met jou',
'text' => 'met jou?',
'x' => '101.42857',
'y' => '368.64789',
'font-size' => '90%',
Expand All @@ -223,7 +223,7 @@ class SvgFileTest extends TestCase
],
'tlh-ca' =>
[
'text' => 'met jou',
'text' => 'met jou?',
'x' => '101.42857',
'y' => '368.64789',
'font-size' => '90%',
Expand All @@ -232,68 +232,19 @@ class SvgFileTest extends TestCase
],
'fallback' =>
[
'text' => ' you',
'text' => ' you?',
'x' => '101.42857',
'y' => '368.64789',
'id' => 'tspan2999',
'sodipodi:role' => 'line',
'data-parent' => 'text2995',
],
],
'text2995' =>
[
'fr' =>
[
'text' => '$1$2?',
'xml:space' => 'preserve',
'x' => '101.42857',
'y' => '318.64789',
'id' => 'text2995-fr',
'sodipodi:linespacing' => '125%',
'data-children' => 'tspan2997|tspan2999',
],
'nl' =>
[
'text' => '$1$2?',
'xml:space' => 'preserve',
'x' => '101.42857',
'y' => '318.64789',
'id' => 'text2995-nl',
'sodipodi:linespacing' => '125%',
'data-children' => 'tspan2997|tspan2999',
],
'tlh-ca' =>
[
'text' => '$1$2?',
'xml:space' => 'preserve',
'x' => '101.42857',
'y' => '318.64789',
'id' => 'text2995-nl',
'sodipodi:linespacing' => '125%',
'data-children' => 'tspan2997|tspan2999',
],
'fallback' =>
[
'text' => '$1$2?',
'xml:space' => 'preserve',
'x' => '101.42857',
'y' => '318.64789',
'id' => 'text2995',
'sodipodi:linespacing' => '125%',
'data-children' => 'tspan2997|tspan2999',
],
],
];

/**
* @var SvgFile
*/
private $svg;

public function setUp(): void
private function getSvg(string $fileName = 'Speech_bubbles.svg'): SvgFile
{
parent::setUp();
$this->svg = new SvgFile(self::TEST_FILE);
return new SvgFile(__DIR__."/../../data/$fileName");
}

/*
Expand All @@ -313,25 +264,26 @@ public function testArrayToNodeToArray(): void
$dom = new DOMDocument('1.0', 'UTF-8');
$dom->loadXML('<text id="tspan2987-de" font-weight="bold">Hallo!</text>');

$node = $this->svg->arrayToNode($array, 'tspan');
$this->assertEquals($this->svg->nodeToArray($dom->firstChild), $this->svg->nodeToArray($node));
$svg = $this->getSvg();
$node = $svg->arrayToNode($array, 'tspan');
$this->assertEquals($svg->nodeToArray($dom->firstChild), $svg->nodeToArray($node));

$expectedArray = $array;
unset($expectedArray['data-parent'], $expectedArray['non-existent']);
$this->assertEquals($expectedArray, $this->svg->nodeToArray($node));
$this->assertEquals($expectedArray, $svg->nodeToArray($node));
}

public function testGetInFileTranslations(): void
{
$this->assertEquals(self::EXPECTED_TRANSLATIONS, $this->svg->getInFileTranslations());
$this->assertEquals(self::EXPECTED_TRANSLATIONS, $this->getSvg()->getInFileTranslations());
}

public function testGetSavedLanguages(): void
{
$expected = [
'de', 'fr', 'nl', 'tlh-ca', 'fallback',
];
$this->assertEquals($expected, $this->svg->getSavedLanguages());
$this->assertEquals($expected, $this->getSvg()->getSavedLanguages());
}

public function testGetSavedLanguagesFiltered(): void
Expand All @@ -340,7 +292,7 @@ public function testGetSavedLanguagesFiltered(): void
'full' => [ 'fr', 'nl', 'tlh-ca', 'fallback' ],
'partial' => [ 'de' ],
];
$this->assertEquals($expected, $this->svg->getSavedLanguagesFiltered());
$this->assertEquals($expected, $this->getSvg()->getSavedLanguagesFiltered());
}

public function testGetFilteredTextNodes(): void
Expand Down Expand Up @@ -457,36 +409,80 @@ public function testGetFilteredTextNodes(): void
'data-children' => 'tspan2991|tspan2993',
],
],
'text2995' => [
'fr' =>
[
'text' => '$1$2',
'xml:space' => 'preserve',
'x' => '101.42857',
'y' => '318.64789',
'id' => 'text2995-fr',
'sodipodi:linespacing' => '125%',
'data-children' => 'tspan2997|tspan2999',
],
'nl' =>
[
'text' => '$1$2',
'xml:space' => 'preserve',
'x' => '101.42857',
'y' => '318.64789',
'id' => 'text2995-nl',
'sodipodi:linespacing' => '125%',
'data-children' => 'tspan2997|tspan2999',
],
'tlh-ca' =>
[
'text' => '$1$2',
'xml:space' => 'preserve',
'x' => '101.42857',
'y' => '318.64789',
'id' => 'text2995-nl',
'sodipodi:linespacing' => '125%',
'data-children' => 'tspan2997|tspan2999',
],
'fallback' =>
[
'text' => '$1$2',
'xml:space' => 'preserve',
'x' => '101.42857',
'y' => '318.64789',
'id' => 'text2995',
'sodipodi:linespacing' => '125%',
'data-children' => 'tspan2997|tspan2999',
],
],
];
$this->assertEquals($expected, $this->svg->getFilteredTextNodes());
$this->assertEquals($expected, $this->getSvg()->getFilteredTextNodes());
}

public function testSwitchTranslationSetRoundtrip(): void
{
// Functions already tested above
$origXml = $this->svg->saveToString();
$current = $this->svg->getInFileTranslations();
$filteredTextNodes = $this->svg->getFilteredTextNodes();
$ret = $this->svg->switchToTranslationSet(array_merge($current, $filteredTextNodes));
$svg = $this->getSvg();
$origXml = $svg->saveToString();
$current = $svg->getInFileTranslations();
$filteredTextNodes = $svg->getFilteredTextNodes();
$ret = $svg->switchToTranslationSet(array_merge($current, $filteredTextNodes));

$this->assertEquals($current, $this->svg->getInFileTranslations());
$this->assertEquals($filteredTextNodes, $this->svg->getFilteredTextNodes());
$this->assertEquals($current, $svg->getInFileTranslations());
$this->assertEquals($filteredTextNodes, $svg->getFilteredTextNodes());
$this->assertEquals([ 'started' => [], 'expanded' => [] ], $ret);

$this->assertEquals(str_replace(' ', '', $origXml), str_replace(' ', '', $this->svg->saveToString()));
$this->assertEquals(str_replace(' ', '', $origXml), str_replace(' ', '', $svg->saveToString()));
}

public function testSetTranslations(): void
{
// The test file does not contain Spanish.
$this->svg->setTranslations('es', ['tspan2993' => 'FooBarX']);
static::assertContains('FooBarX', $this->svg->saveToString());
$svg = $this->getSvg();
$svg->setTranslations('es', ['tspan2993' => 'FooBarX']);
static::assertContains('FooBarX', $svg->saveToString());

// Test that multiple languages can be set, and modified.
$this->svg->setTranslations('es', ['tspan2993' => 'FooBarModified']);
$this->svg->setTranslations('fr', ['tspan2993' => 'BarFoo']);
$svg->setTranslations('es', ['tspan2993' => 'FooBarModified']);
$svg->setTranslations('fr', ['tspan2993' => 'BarFoo']);

$svgContent = $this->svg->saveToString();
$svgContent = $svg->saveToString();
static::assertContains('FooBarModified', $svgContent);
static::assertContains('BarFoo', $svgContent);
static::assertNotContains('FooBarX', $svgContent);
Expand All @@ -495,8 +491,9 @@ public function testSetTranslations(): void
public function testT214717(): void
{
$fileName = tempnam(sys_get_temp_dir(), 'SvgFile');
$this->svg->setTranslations('ru', ['tspan2993' => '']);
$this->svg->saveToPath($fileName);
$svg = $this->getSvg();
$svg->setTranslations('ru', ['tspan2993' => '']);
$svg->saveToPath($fileName);

$newSvg = new SvgFile($fileName);
unlink($fileName);
Expand All @@ -507,27 +504,27 @@ public function testT214717(): void
public function testSaveToString(): void
{
// Check that we are not actually destroying the XML file
$this->assertGreaterThan(1500, strlen($this->svg->saveToString()));
$this->assertGreaterThan(1500, strlen($this->getSvg()->saveToString()));
}

public function testSaveToPath(): void
{
$tempPath = tempnam(sys_get_temp_dir(), 'test');
$this->svg->saveToPath($tempPath);
$this->getSvg()->saveToPath($tempPath);

// Check that we are not actually destroying the XML file
$this->assertGreaterThan(1500, strlen(file_get_contents($tempPath)));
}

public function testEmptySvg(): void
{
$file = new SvgFile(__DIR__.'/../../data/empty.svg');
$file = $this->getSvg('empty.svg');
$this->assertEquals([], $file->getInFileTranslations());
}

public function testUnevenTspans(): void
{
$file = new SvgFile(__DIR__.'/../../data/tspans.svg');
$file = $this->getSvg('tspans.svg');
$this->assertEquals(
[
'trsvg3' => [
Expand All @@ -552,4 +549,14 @@ public function testUnevenTspans(): void
$file->getInFileTranslations()
);
}

/**
* https://phabricator.wikimedia.org/T216283
* Handle some parts of a <text> being <tspan> and some not
*/
public function testMixedTextContent(): void
{
$svg = $this->getSvg('mixed.svg');
$this->assertCount(6, $svg->getInFileTranslations());
}
}
7 changes: 3 additions & 4 deletions tests/data/Speech_bubbles.svg
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions tests/data/mixed.svg
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit cada922

Please sign in to comment.