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 <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Jarry1250 <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
