Use original IDs if possible when extracting XLIFF strings

app/classes/Transvision/Xliff.php

@@ -38,7 +38,7 @@ public static function getStrings($xliff_path, $relative_file, $project_name, $r
                 $file_node = $trans_unit->xpath('../..');
                 $file_orig = $file_node[0]['original'];
 
-                $string_id = self::generateStringID($project_name, $file_name, $file_orig, $trans_unit['id']);
+                $string_id = self::generateStringID($project_name, $file_name, $file_orig, $trans_unit['id'], $strings);
 
                 if ($reference_locale) {
                     // If it's the reference locale, we use the source instead of the target
@@ -60,18 +60,36 @@ public static function getStrings($xliff_path, $relative_file, $project_name, $r
 
     /**
      * Generate a unique ID for a string to store in Transvision.
-     * String ID can be identical to the source string in iOS, so it's more
-     * reliable to generate a unique ID from it.
+     *
+     * If the string ID includes a space, assume it's text, and generate a
+     * unique identifier based on the file and ID. If there is already a string
+     * with that ID extracted, add the hash.
+     *
      *
      * @param string $project_name The project this string belongs to
      * @param string $file_name    .xliff file name
      * @param string $file_orig    'original' attribute of the element's parent
      * @param string $string_id    'id' attribute of the <trans-unit> element
+     * @param string $strings      strings already extracted from the file
      *
-     * @return string unique ID such as firefox_ios/firefox-ios.xliff:1dafea7725862ca854c408f0e2df9c88
+     * @return string Either string ID, or unique ID based on the file and ID
+     *                firefox_ios/firefox-ios.xliff:1dafea7725862ca854c408f0e2df9c88
      */
-    public static function generateStringID($project_name, $file_name, $file_orig, $string_id)
+    public static function generateStringID($project_name, $file_name, $file_orig, $string_id, $strings)
     {
-        return "{$project_name}/{$file_name}:" . hash('md5', $file_orig . $string_id);
+        // If $string_id contains a space, generate unique ID.
+        if (strpos($string_id, ' ') !== false) {
+            return "{$project_name}/{$file_name}:" . hash('md5', $file_orig . $string_id);
+        }
+
+        $generated_id = "{$project_name}/{$file_name}:{$string_id}";
+
+        // If we already extracted a string with the same ID, use a more unique ID.
+        if (array_key_exists($generated_id, $strings)) {
+            // If already defined, fall back to the generated ID.
+            return "{$generated_id}-" . hash('md5', $file_orig . $string_id);
+        }
+
+        return $generated_id;
     }
 }

tests/units/Transvision/XliffTest.php

@@ -27,10 +27,10 @@ public function testGetStrings()
 
         // Check escaped single straight quotes
         $this
-            ->assertSame($strings['firefox_ios/firefox-ios.xliff:e15c1a9a6082aa32623205328418a603'], "Test con \'");
+            ->assertSame($strings['firefox_ios/firefox-ios.xliff:test_escape'], "Test con \'");
 
         $this
-            ->assertSame($strings['firefox_ios/firefox-ios.xliff:1348465d2e7136641805937598daaeda'], "Test con \\\\\' già escaped");
+            ->assertSame($strings['firefox_ios/firefox-ios.xliff:test_already_escaped'], "Test con \\\\\' già escaped");
     }
 
     public function testGetStringsReference()
@@ -51,10 +51,10 @@ public function testGetStringsReference()
 
         // Check escaped single straight quotes
         $this
-            ->assertSame($strings['firefox_ios/firefox-ios.xliff:e15c1a9a6082aa32623205328418a603'], "Test with \'");
+            ->assertSame($strings['firefox_ios/firefox-ios.xliff:test_escape'], "Test with \'");
 
         $this
-            ->assertSame($strings['firefox_ios/firefox-ios.xliff:1348465d2e7136641805937598daaeda'], "Test with \\\\\' already escaped");
+            ->assertSame($strings['firefox_ios/firefox-ios.xliff:test_already_escaped'], "Test with \\\\\' already escaped");
     }
 
     public static function generateStringID_DP()
@@ -65,30 +65,43 @@ public static function generateStringID_DP()
                 'firefox-ios.xliff',
                 'AccountTests/Info.plist',
                 'Delete',
-                'firefox_ios/firefox-ios.xliff:bb46926d0fcd6d43155f706a22b0f3fc',
+                [],
+                'firefox_ios/firefox-ios.xliff:Delete',
             ],
             [
                 'firefox_ios',
                 'firefox-ios.xliff',
                 'DiffentFile/Info.plist',
                 'Delete',
-                'firefox_ios/firefox-ios.xliff:e3b9ee7a5b6b4e96f70c539d87aff9b0',
+                [],
+                'firefox_ios/firefox-ios.xliff:Delete',
+            ],
+            [
+                'firefox_ios',
+                'firefox-ios.xliff',
+                'DiffentFile/Info.plist',
+                'Delete',
+                [
+                    'firefox_ios/firefox-ios.xliff:Delete' => 'Some Test',
+                ],
+                'firefox_ios/firefox-ios.xliff:Delete-e3b9ee7a5b6b4e96f70c539d87aff9b0',
             ],
             [
                 'firefox_ios',
                 'firefox-ios.xliff',
                 'Client/3DTouchActions.strings',
                 'Are you sure you want to clear all of your data? This will also close all open tabs.',
+                [],
                 'firefox_ios/firefox-ios.xliff:46e4ec3c64a0ce5a5d9c5f8bebd74325',
             ],
         ];
     }
 
     #[DataProvider('generateStringID_DP')]
-     public function testGenerateStringID($a, $b, $c, $d, $e)
+     public function testGenerateStringID($a, $b, $c, $d, $e, $f)
     {
         $obj = new Xliff();
         $this
-            ->assertSame($obj->generateStringID($a, $b, $c, $d), $e);
+            ->assertSame($obj->generateStringID($a, $b, $c, $d, $e), $f);
     }
 }