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

Reply via email to