Fix double escaping in XLIFF (#1011)

mozfr · Apr 29, 2021 · 16b6178 · 16b6178
1 parent a2d2e5d
commit 16b6178
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 6 deletions.
diff --git a/app/classes/Transvision/Xliff.php b/app/classes/Transvision/Xliff.php
@@ -32,6 +32,7 @@ public static function getStrings($xliff_path, $project_name, $reference_locale
                 translation (target).
             */
             $trans_units = $xml->xpath('//x:trans-unit');
+
             foreach ($trans_units as $trans_unit) {
                 $file_node = $trans_unit->xpath('../..');
                 $file_orig = $file_node[0]['original'];
@@ -40,16 +41,15 @@ public static function getStrings($xliff_path, $project_name, $reference_locale
 
                 if ($reference_locale) {
                     // If it's the reference locale, we use the source instead of the target
-                    $translation = str_replace("'", "\\'", $trans_unit->source);
-                    $strings[$string_id] = $translation;
+
+                    $strings[$string_id] = addslashes($trans_unit->source);
                 } elseif (isset($trans_unit->target)) {
                     /*
                         We only store the translation if the target is set.
                         simplexml returns an empty string if the element is
                         missing.
                     */
-                    $translation = str_replace("'", "\\'", $trans_unit->target);
-                    $strings[$string_id] = $translation;
+                    $strings[$string_id] = addslashes($trans_unit->target);
                 }
             }
         }

diff --git a/tests/testfiles/xliff/firefox-ios.xliff b/tests/testfiles/xliff/firefox-ios.xliff
@@ -73,6 +73,14 @@
         <target>Accessi salvati</target>
         <note>Settings item for clearing passwords and login data</note>
       </trans-unit>
+      <trans-unit id="test_escape">
+        <source>Test with '</source>
+        <target>Test con '</target>
+      </trans-unit>
+      <trans-unit id="test_already_escaped">
+        <source>Test with \' already escaped</source>
+        <target>Test con \' già escaped</target>
+      </trans-unit>
     </body>
   </file>
   <file original="Client/ClearPrivateData2.strings" source-language="en" datatype="plaintext" target-language="it">

diff --git a/tests/units/Transvision/Xliff.php b/tests/units/Transvision/Xliff.php
@@ -16,7 +16,7 @@ public function testGetStrings()
         // Check total number of strings
         $this
             ->integer(count($strings))
-                ->isEqualTo(13);
+                ->isEqualTo(15);
 
         // Check strings
         $this
@@ -26,6 +26,15 @@ public function testGetStrings()
         $this
             ->string($strings['firefox_ios/firefox-ios.xliff:f6b6d1aff4ade5b867d563d74b01a429'])
                 ->isEqualTo('Segnalibri pc desktop');
+
+        // Check escaped single straight quotes
+        $this
+            ->string($strings['firefox_ios/firefox-ios.xliff:e15c1a9a6082aa32623205328418a603'])
+                ->isEqualTo("Test con \'");
+
+        $this
+            ->string($strings['firefox_ios/firefox-ios.xliff:1348465d2e7136641805937598daaeda'])
+                ->isEqualTo("Test con \\\\\' già escaped");
     }
 
     public function testGetStringsReference()
@@ -36,7 +45,7 @@ public function testGetStringsReference()
         // Check total number of strings
         $this
             ->integer(count($strings))
-                ->isEqualTo(14);
+                ->isEqualTo(16);
 
         // Check strings
         $this
@@ -46,6 +55,15 @@ public function testGetStringsReference()
         $this
             ->string($strings['firefox_ios/firefox-ios.xliff:f6b6d1aff4ade5b867d563d74b01a429'])
                 ->isEqualTo('Desktop Bookmarks');
+
+        // Check escaped single straight quotes
+        $this
+            ->string($strings['firefox_ios/firefox-ios.xliff:e15c1a9a6082aa32623205328418a603'])
+                ->isEqualTo("Test with \'");
+
+        $this
+            ->string($strings['firefox_ios/firefox-ios.xliff:1348465d2e7136641805937598daaeda'])
+                ->isEqualTo("Test with \\\\\' already escaped");
     }
 
     public function generateStringID_DP()