Fail earlier for multiple-text-same-lang structure

It's not permitted to have multiple `<text>` elements with the same language
code in a `<switch>` element. This wasn't being checked in
`SvgFile::makeTranslationRead()`, but only later in
`SvgFile::switchToTranslationSet()`, and so it wasn't being reported to the
user when they opened a file for translation, but only later when they tried to
load a translated version (at which point it failed without telling them why).

This change moves the multiple-language check to the earlier method, and fixes
up some broken parts of 91d3097 such as the
error message name and parameters, as well as i18n messages and documentation.

This doesn't remove the later check, even though it's now redundant. That can
be done in a follow-up.

Bug: T319154

i18n/en.json

@@ -30,7 +30,7 @@
 
   "structure-error-no-doc-element": "No document element found.",
   "structure-error-nested-tspans-not-supported": "This file can not be translated because it contains nested tspan elements in $1.",
-  "structure-error-multiple-text-same-lang": "Multiple text elements found with language '$1'",
+  "structure-error-multiple-text-same-lang": "Multiple text elements found with language code '$2' (in element with ID '$1').",
   "structure-error-contains-tref": "This file contains tref tags, which are not supported by this tool.",
   "structure-error-css-too-complex": "This file contains CSS that is too complicated to parse.",
   "structure-error-css-has-ids": "This file uses element IDs in the CSS, which may break when SVG Translate adds new IDs. It should use classes instead, if possible.",

i18n/qqq.json

@@ -43,7 +43,7 @@
 	"structure-error-css-has-ids": "Message displayed when an SVG file has an error that renders it untranslatable.",
 	"structure-error-unexpected-node-in-text": "Message displayed when an SVG file has an error that renders it untranslatable.",
 	"structure-error-invalid-node-id": "Message displayed when an SVG file has an error that renders it untranslatable.\n\nParameters:\n* $1 — Closest parent element ID.",
-	"structure-error-text-contains-dollar": "Message displayed when an SVG file has an error that renders it untranslatable.\n\nParameters:\n* $1 — Closest parent element ID.",
+	"structure-error-text-contains-dollar": "Message displayed when an SVG file has an error that renders it untranslatable.\n\nParameters:\n* $1 — Closest parent element ID.\n* $2 — Offending text content.",
 	"structure-error-non-tspan-inside-text": "Message displayed when an SVG file has an error that renders it untranslatable.",
 	"structure-error-switch-text-is-not-node": "Message displayed when an SVG file has an error that renders it untranslatable.",
 	"structure-error-switch-text-content-outside-text": "Message displayed when an SVG file has an error that renders it untranslatable.",

src/Controller/TranslateController.php

@@ -80,9 +80,10 @@ public function translate(
             return $this->showError('network-error', $normalizedFilename);
         } catch (SvgStructureException $exception) {
             $msgParams = $exception->getMessageParams();
-            // Get the element ID, or fall back to the no-ID placeholder message.
-            $id = $exception->getClosestId();
-            array_unshift($msgParams, $id ?: $intuition->msg('structure-error-no-id'));
+            // If there's no element ID fall back to the no-ID placeholder message.
+            if ($msgParams[0] === null) {
+                $msgParams[0] = $intuition->msg('structure-error-no-id');
+            }
             return $this->showError(
                 $exception->getMessage(),
                 $normalizedFilename,

src/Model/Svg/SvgFile.php

@@ -274,10 +274,11 @@ protected function makeTranslationReady(): void
             }
         }
 
-        $textLength = $this->document->getElementsByTagName('text')->length;
+        $texts = $this->document->getElementsByTagName('text');
+        $textLength = $texts->length;
         for ($i = 0; $i < $textLength; $i++) {
             /** @var DOMElement $text */
-            $text = $this->document->getElementsByTagName('text')->item($i);
+            $text = $texts->item($i);
 
             // Text strings like $1, $2 will cause problems later because
             // self::replaceIndicesRecursive() will try to replace them
@@ -325,6 +326,7 @@ protected function makeTranslationReady(): void
         for ($i = 0; $i < $switchLength; $i++) {
             $switch = $this->document->getElementsByTagName('switch')->item($i);
             $siblings = $switch->childNodes;
+            $existingLangs = [];
             foreach ($siblings as $sibling) {
                 /** @var DOMElement $sibling */
 
@@ -344,6 +346,14 @@ protected function makeTranslationReady(): void
                     throw new SvgStructureException('structure-error-switch-child-not-text', $sibling);
                 }
 
+                // Make sure there's not more than one text element with the same lang.
+                $textLang = $sibling->getAttribute('systemLanguage');
+                if ( in_array( $textLang, $existingLangs ) ) {
+                    throw new SvgStructureException('structure-error-multiple-text-same-lang', $switch, [$textLang]);
+                }
+                $existingLangs[] = $textLang;
+
+                // Check for comma-separated language codes (and reject the file if any duplicates are found).
                 $language = $sibling->hasAttribute('systemLanguage') ?
                     $sibling->getAttribute('systemLanguage') : 'fallback';
                 $realLangs = preg_split('/, */', $language);
@@ -616,8 +626,9 @@ public function switchToTranslationSet(array $translations): array
                     // No matching text node for this language, so we'll create one
                     $switch->appendChild($newTextTag);
                 } else {
-                    // If there is more than existing text element in a switch (with the given lang), give up on this file.
-                    throw new SvgStructureException('multiple-text-same-lang', $switch, [$language]);
+                    // If there is more than one existing text element in a switch (with the given lang), give up on this file.
+                    // This should never happen, because this same check is done in makeTranslationReady().
+                    throw new SvgStructureException('structure-error-multiple-text-same-lang', $switch, [$language]);
                 }
 
                 // To have got this far, we must have either updated or started a new language

tests/Model/Svg/SvgFileTest.php

@@ -774,7 +774,7 @@ public function testAddsTextToSwitch() {
                 . '</svg>');
                 $svgFile3->setTranslations('la', ['trsvg3' => 'lang la (new)']);
         } catch (SvgStructureException $exception) {
-            $this->assertSame('multiple-text-same-lang', $exception->getMessage());
+            $this->assertSame('structure-error-multiple-text-same-lang', $exception->getMessage());
             $this->assertSame(['testswitch', 'la'], $exception->getMessageParams());
         }
     }