Jarry1250 has submitted this change and it was merged. Change subject: Move language splitting logic from analyse() to makeTranslationReady() ......................................................................
Move language splitting logic from analyse() to makeTranslationReady() Although the SVG spec supports multi-language text tags (e.g. "en,fr,de") these are a really poor idea since (a) they are confusing to read and (b) the desired translations could diverge at any point. So get rid at the earliest possible juncture, i.e. in makeTranslationReady(). Fix associated tests that relied on order of <text> elements. Change-Id: I9a0aa022315e38e7cb7eae0563d05cf4837de637 --- M SVGFile.php M tests/phpunit/SVGFileTest.php 2 files changed, 37 insertions(+), 23 deletions(-) Approvals: Jarry1250: Verified; Looks good to me, approved jenkins-bot: Verified diff --git a/SVGFile.php b/SVGFile.php index 7bc4908..39bacad 100644 --- a/SVGFile.php +++ b/SVGFile.php @@ -203,11 +203,27 @@ } $language = $sibling->hasAttribute( 'systemLanguage' ) ? $sibling->getAttribute( 'systemLanguage' ) : 'fallback'; - if ( in_array( $language, $languagesPresent ) ) { - // Two tags for the same language - return false; + $realLangs = preg_split( '/, */', $language ); + foreach( $realLangs as $realLang ) { + if( count( $realLangs ) > 1 ) { + // Although the SVG spec supports multi-language text tags (e.g. "en,fr,de") + // these are a really poor idea since (a) they are confusing to read and (b) the + // desired translations could diverge at any point. So get rid. + $singleLanguageNode = clone $sibling; + $singleLanguageNode->setAttribute( 'systemLanguage', $realLang ); + $switch->appendChild( $singleLanguageNode ); + } + if ( in_array( $realLang, $languagesPresent ) ) { + // Two tags for the same language + return false; + } + $languagesPresent[] = $realLang; } - $languagesPresent[] = $language; + + if( count( $realLangs ) > 1 ) { + // If still present, remove the original multi-language + $switch->removeChild( $sibling ); + } } } } else { @@ -288,8 +304,7 @@ $numChildren = $text->childNodes->length; $hasActualTextContent = TranslateSvgUtils::hasActualTextContent( $text ); $lang = $text->hasAttribute( 'systemLanguage' ) ? $text->getAttribute( 'systemLanguage' ) : 'fallback'; - $realLangs = preg_split( '/, */', $lang ); - $realLangs = array_map( 'TranslateSvgUtils::osToLangCode', $realLangs ); + $langCode = TranslateSvgUtils::osToLangCode( $lang ); $counter = 1; for ( $k = 0; $k < $numChildren; $k++ ) { @@ -302,10 +317,8 @@ $childTspan = $fallbackText->getElementsByTagName( 'tspan' )->item( $counter - 1 ); $childId = $childTspan->getAttribute( 'id' ); - foreach( $realLangs as $realLang ) { - $translations[$childId][$realLang] = TranslateSvgUtils::nodeToArray( $child ); - $translations[$childId][$realLang]['data-parent'] = $textId; - } + $translations[$childId][$langCode] = TranslateSvgUtils::nodeToArray( $child ); + $translations[$childId][$langCode]['data-parent'] = $textId; if ( $text->hasAttribute( 'data-children' ) ) { $existing = $text->getAttribute( 'data-children' ); $text->setAttribute( 'data-children', "$existing|$childId" ); @@ -318,17 +331,15 @@ $counter++; } } - foreach( $realLangs as $realLang ) { - if ( $hasActualTextContent ) { - // If the <text> has *its own* text content, rather than just <tspan>s, register it - // for translation. - $translations[$textId][$realLang] = TranslateSvgUtils::nodeToArray( $text ); - } else { - $this->filteredTextNodes[$textId][$realLang] = TranslateSvgUtils::nodeToArray( $text ); - } - $savedLang = ( $realLang === 'fallback' ) ? $this->fallbackLanguage : $realLang; - $this->savedLanguages[] = $savedLang; + if ( $hasActualTextContent ) { + // If the <text> has *its own* text content, rather than just <tspan>s, register it + // for translation. + $translations[$textId][$langCode] = TranslateSvgUtils::nodeToArray( $text ); + } else { + $this->filteredTextNodes[$textId][$langCode] = TranslateSvgUtils::nodeToArray( $text ); } + $savedLang = ( $langCode === 'fallback' ) ? $this->fallbackLanguage : $langCode; + $this->savedLanguages[] = $savedLang; } } $this->inFileTranslations = $translations; @@ -461,7 +472,10 @@ "svg:text[@systemLanguage='$language']|text[@systemLanguage='$language']"; $existing = $this->xpath->query( $path, $switch ); if ( $existing->length == 1 ) { - // Only one matching text node, replace + // Only one matching text node, replace if different + if ( TranslateSvgUtils::nodeToArray( $newTextTag ) === TranslateSvgUtils::nodeToArray( $existing->item( 0 ) ) ) { + continue; + } $switch->replaceChild( $newTextTag, $existing->item( 0 ) ); } elseif ( $existing->length == 0 ) { // No matching text node for this language, so we'll create one diff --git a/tests/phpunit/SVGFileTest.php b/tests/phpunit/SVGFileTest.php index 31727c8..ec83b0c 100644 --- a/tests/phpunit/SVGFileTest.php +++ b/tests/phpunit/SVGFileTest.php @@ -281,14 +281,14 @@ public function testGetSavedLanguages() { $expected = array( - 'de', 'fr', 'nl', 'tlh-ca', 'en' + 'de', 'fr', 'en', 'nl', 'tlh-ca' ); $this->assertEquals( $expected, $this->svg->getSavedLanguages() ); } public function testGetSavedLanguagesFiltered() { $expected = array( - 'full' => array( 'fr', 'nl', 'tlh-ca', 'en' ), + 'full' => array( 'fr', 'en', 'nl', 'tlh-ca' ), 'partial' => array( 'de' ) ); $this->assertEquals( $expected, $this->svg->getSavedLanguagesFiltered() ); -- To view, visit https://gerrit.wikimedia.org/r/156133 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I9a0aa022315e38e7cb7eae0563d05cf4837de637 Gerrit-PatchSet: 3 Gerrit-Project: mediawiki/extensions/TranslateSvg Gerrit-Branch: master Gerrit-Owner: Jarry1250 <jarry1...@gmail.com> Gerrit-Reviewer: Addshore <addshorew...@gmail.com> Gerrit-Reviewer: Jarry1250 <jarry1...@gmail.com> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits