tor-browser

commit 43612cbc8603f0ce86502f9075b0ab96aaebe48e
parent ba860649ac0fe6bafcaf41c9cc7959991679346e
Author: Rob Wu <rob@robwu.nl>
Date:   Thu,  9 Oct 2025 14:55:57 +0000

Bug 1993463 - Include l10n ID in l10n-args errors + tests r=eemeli

Differential Revision: https://phabricator.services.mozilla.com/D268126

Diffstat:
M
dom/l10n/DOMLocalization.cpp
 | 
16
+++++++++-------
M
dom/l10n/DOMLocalization.h
 | 
4
++--
M
dom/l10n/tests/mochitest/dom_localization/test_getAttributes.html
 | 
43
+++++++++++++++++++++++++++++++++++++++++++

3 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/dom/l10n/DOMLocalization.cpp b/dom/l10n/DOMLocalization.cpp
@@ -172,7 +172,8 @@ void DOMLocalization::GetAttributes(Element& aElement, L10nIdArgs& aResult,
   }
 
   if (aElement.GetAttr(nsGkAtoms::datal10nargs, l10nArgs)) {
-    ConvertStringToL10nArgs(l10nArgs, aResult.mArgs.SetValue(), aRv);
+    ConvertStringToL10nArgs(aResult.mId, l10nArgs, aResult.mArgs.SetValue(),
+                            aRv);
   }
 }
 
@@ -640,7 +641,8 @@ void DOMLocalization::ReportL10nOverlaysErrors(
   }
 }
 
-void DOMLocalization::ConvertStringToL10nArgs(const nsString& aInput,
+void DOMLocalization::ConvertStringToL10nArgs(const nsCString& aL10nId,
+                                              const nsString& aInput,
                                               intl::L10nArgs& aRetVal,
                                               ErrorResult& aRv) {
   if (aInput.IsEmpty()) {
@@ -653,7 +655,7 @@ void DOMLocalization::ConvertStringToL10nArgs(const nsString& aInput,
 
   if (!jsonReader.parse(NS_ConvertUTF16toUTF8(aInput).get(), args, false)) {
     nsTArray<nsCString> errors{
-        "[dom/l10n] Failed to parse l10n-args JSON: "_ns +
+        "[dom/l10n] Failed to parse l10n-args JSON ("_ns + aL10nId + "): "_ns +
             NS_ConvertUTF16toUTF8(aInput),
     };
     MaybeReportErrorsToGecko(errors, aRv, GetParentObject());
@@ -662,8 +664,8 @@ void DOMLocalization::ConvertStringToL10nArgs(const nsString& aInput,
 
   if (!args.isObject()) {
     nsTArray<nsCString> errors{
-        "[dom/l10n] Failed to parse l10n-args JSON: "_ns +
-            NS_ConvertUTF16toUTF8(aInput),
+        "[dom/l10n] Failed to parse l10n-args as JSON object ("_ns + aL10nId +
+            "): "_ns + NS_ConvertUTF16toUTF8(aInput),
     };
     MaybeReportErrorsToGecko(errors, aRv, GetParentObject());
     return;
@@ -692,8 +694,8 @@ void DOMLocalization::ConvertStringToL10nArgs(const nsString& aInput,
       newEntry->mValue.SetNull();
     } else {
       nsTArray<nsCString> errors{
-          "[dom/l10n] Failed to convert l10n-args JSON: "_ns +
-              NS_ConvertUTF16toUTF8(aInput),
+          "[dom/l10n] Failed to convert l10n-args JSON ("_ns + aL10nId +
+              "): "_ns + NS_ConvertUTF16toUTF8(aInput),
       };
       MaybeReportErrorsToGecko(errors, aRv, GetParentObject());
     }
diff --git a/dom/l10n/DOMLocalization.h b/dom/l10n/DOMLocalization.h
@@ -125,8 +125,8 @@ class DOMLocalization : public intl::Localization {
   void DisconnectMutations();
   void DisconnectRoots();
   void ReportL10nOverlaysErrors(nsTArray<L10nOverlaysError>& aErrors);
-  void ConvertStringToL10nArgs(const nsString& aInput, intl::L10nArgs& aRetVal,
-                               ErrorResult& aRv);
+  void ConvertStringToL10nArgs(const nsCString& aL10nId, const nsString& aInput,
+                               intl::L10nArgs& aRetVal, ErrorResult& aRv);
 
   RefPtr<L10nMutations> mMutations;
   nsTHashSet<RefPtr<nsINode>> mRoots;
diff --git a/dom/l10n/tests/mochitest/dom_localization/test_getAttributes.html b/dom/l10n/tests/mochitest/dom_localization/test_getAttributes.html
@@ -8,6 +8,15 @@
   <script type="application/javascript">
   "use strict";
 
+  function assertThrows(fn, expected, msg) {
+    try {
+      fn();
+      ok(false, `Error unexpectedly NOT thrown: ${expected} (${msg})`);
+    } catch (e) {
+      is(e.message, expected, msg);
+    }
+  }
+
   window.onload = function() {
     SimpleTest.waitForExplicitFinish();
 
@@ -19,6 +28,10 @@
     const p1 = document.querySelectorAll("p")[0];
     const p2 = document.querySelectorAll("p")[1];
     const p3 = document.querySelectorAll("p")[2];
+    const p4 = document.querySelectorAll("p")[3];
+    const p5 = document.querySelectorAll("p")[4];
+    const p6 = document.querySelectorAll("p")[5];
+    const p7 = document.querySelectorAll("p")[6];
     const attrs1 = domLoc.getAttributes(p1);
     const attrs2 = domLoc.getAttributes(p2);
     const attrs3 = domLoc.getAttributes(p3);
@@ -37,6 +50,32 @@
       },
     });
 
+    // Note: In tests these errors are thrown, but in production they are
+    // merely causing logspam (bug 1741430).
+    assertThrows(
+      () => domLoc.getAttributes(p4),
+      "DOMLocalization.getAttributes: [dom/l10n] Failed to parse l10n-args JSON (id3): [not a JSON obj]",
+      "Expected error when data-l10n-args cannot be parsed as JSON"
+    );
+
+    assertThrows(
+      () => domLoc.getAttributes(p5),
+      "DOMLocalization.getAttributes: [dom/l10n] Failed to parse l10n-args as JSON object (id4): 123456",
+      "Expected error when data-l10n-args is not a JSON object"
+    );
+
+    assertThrows(
+      () => domLoc.getAttributes(p6),
+      "DOMLocalization.getAttributes: [dom/l10n] Failed to convert l10n-args JSON (id5): {\"a\":1,\"arr\":[],\"c\":2}",
+      "Expected error when data-l10n-args contains array members"
+    );
+
+    assertThrows(
+      () => domLoc.getAttributes(p7),
+      "DOMLocalization.getAttributes: [dom/l10n] Failed to parse l10n-args JSON (): invalid JSON and without l10n-id",
+      "Expected error when data-l10n-args is not a JSON object, without l10n-id"
+    );
+
     SimpleTest.finish();
   };
   </script>
@@ -45,5 +84,9 @@
   <p />
   <p data-l10n-id="id1" />
   <p data-l10n-id="id2" data-l10n-args='{"userName": "John"}' />
+  <p data-l10n-id="id3" data-l10n-args="[not a JSON obj]" />
+  <p data-l10n-id="id4" data-l10n-args="123456" />
+  <p data-l10n-id="id5" data-l10n-args='{"a":1,"arr":[],"c":2}' />
+  <p data-l10n-args="invalid JSON and without l10n-id" />
 </body>
 </html>