Bug 1453765 - Switch Fluent from warning to throwing errors when in debug/testing mode. r?gijs draft
authorZibi Braniecki <zbraniecki@mozilla.com>
Tue, 22 May 2018 20:49:29 -0700
changeset 798986 f315a45055e661819721f242d7654a45edbd7661
parent 798691 d36cd8bdbc5c0df1d1d7a167f5fedb95c3a3648e
push id110913
push userbmo:gandalf@aviary.pl
push dateWed, 23 May 2018 21:21:41 +0000
reviewersgijs
bugs1453765
milestone62.0a1
Bug 1453765 - Switch Fluent from warning to throwing errors when in debug/testing mode. r?gijs MozReview-Commit-ID: 1y7FTCPWRxb
browser/components/preferences/in-content/privacy.xul
browser/components/preferences/siteDataSettings.js
intl/l10n/DOMLocalization.jsm
intl/l10n/Localization.jsm
intl/l10n/test/dom/test_domloc_overlay_missing_all.html
--- a/browser/components/preferences/in-content/privacy.xul
+++ b/browser/components/preferences/in-content/privacy.xul
@@ -223,17 +223,17 @@
       </radiogroup>
     </vbox>
     <vbox>
       <!-- Please don't remove the wrapping hbox/vbox/box for these elements. It's used to properly compute the search tooltip position. -->
       <hbox>
         <button id="clearSiteDataButton"
             class="accessory-button"
             icon="clear"
-            search-l10n-ids="clear-site-data-cookies, clear-site-data-cache"
+            search-l10n-ids="clear-site-data-cookies-empty.label, clear-site-data-cache-empty.label"
             data-l10n-id="sitedata-clear"/>
       </hbox>
       <hbox>
         <button id="siteDataSettings"
                 class="accessory-button"
                 data-l10n-id="sitedata-settings"
                 search-l10n-ids="
                   site-data-settings-window.title,
--- a/browser/components/preferences/siteDataSettings.js
+++ b/browser/components/preferences/siteDataSettings.js
@@ -33,17 +33,17 @@ let gSiteDataSettings = {
     // Creates a new column item with the specified relative width.
     function addColumnItem(l10n, flexWidth) {
       let box = document.createElement("hbox");
       box.className = "item-box";
       box.setAttribute("flex", flexWidth);
       let label = document.createElement("label");
       label.setAttribute("crop", "end");
       if (l10n) {
-        if (l10n.raw) {
+        if (l10n.hasOwnProperty("raw")) {
           box.setAttribute("tooltiptext", l10n.raw);
           label.setAttribute("value", l10n.raw);
         } else {
           document.l10n.setAttributes(label, l10n.id, l10n.args);
         }
       }
       box.appendChild(label);
       container.appendChild(box);
--- a/intl/l10n/DOMLocalization.jsm
+++ b/intl/l10n/DOMLocalization.jsm
@@ -606,17 +606,20 @@ class DOMLocalization extends Localizati
       }
     }
 
     // This fragment allows us to coalesce all pending translations
     // into a single requestAnimationFrame.
     if (this.pendingElements.size > 0) {
       if (this.pendingrAF === null) {
         this.pendingrAF = this.windowElement.requestAnimationFrame(() => {
-          this.translateElements(Array.from(this.pendingElements));
+          // We need to filter for elements that lost their l10n-id while
+          // waiting for the animation frame.
+          this.translateElements(Array.from(this.pendingElements)
+            .filter(elem => elem.hasAttribute("data-l10n-id")));
           this.pendingElements.clear();
           this.pendingrAF = null;
         });
       }
     }
   }
 
   /**
@@ -680,17 +683,20 @@ class DOMLocalization extends Localizati
           for (let i = 0; i < overlayTranslations.length; i++) {
             if (overlayTranslations[i] !== undefined &&
                 untranslatedElements[i] !== undefined) {
               translateElement(untranslatedElements[i], overlayTranslations[i]);
             }
           }
           this.resumeObserving();
         })
-        .catch(() => this.resumeObserving());
+        .catch(e => {
+          this.resumeObserving();
+          throw e;
+        });
     }
     return this.translateElements(this.getTranslatables(frag));
   }
 
   /**
    * Translate a list of DOM elements asynchronously using this
    * `DOMLocalization` object.
    *
--- a/intl/l10n/Localization.jsm
+++ b/intl/l10n/Localization.jsm
@@ -143,19 +143,22 @@ class Localization {
 
     for await (const ctx of this.ctxs) {
       const missingIds = keysFromContext(method, ctx, keys, translations);
 
       if (missingIds.size === 0) {
         break;
       }
 
-      if (AppConstants.NIGHTLY_BUILD) {
+      if (AppConstants.NIGHTLY_BUILD || Cu.isInAutomation) {
         const locale = ctx.locales[0];
         const ids = Array.from(missingIds).join(", ");
+        if (Cu.isInAutomation) {
+          throw new Error(`Missing translations in ${locale}: ${ids}`);
+        }
         console.warn(`Missing translations in ${locale}: ${ids}`);
       }
     }
 
     return translations;
   }
 
   /**
--- a/intl/l10n/test/dom/test_domloc_overlay_missing_all.html
+++ b/intl/l10n/test/dom/test_domloc_overlay_missing_all.html
@@ -22,21 +22,21 @@
   addLoadEvent(async () => {
 
     const domLoc = new DOMLocalization(
       window,
       [],
       mockGenerateMessages
     );
 
-    await domLoc.translateFragment(document.body);
-
-    // we just care that it doesn't throw here.
-    ok(1);
-
+    await domLoc.translateFragment(document.body).then(() => {
+      ok(false, "Expected translateFragment to throw on missing l10n-id");
+    }, () => {
+      ok(true, "Expected translateFragment to throw on missing l10n-id");
+    });
     SimpleTest.finish();
   });
   </script>
 </head>
 <body>
   <p data-l10n-id="title">
     <a href="http://www.mozilla.org"></a>
     <a href="http://www.firefox.com"></a>