Bug 1835559 - Ensure that text content survives translation...

Bug 1835559 - Ensure that text content survives translation r=application-update-reviewers,flod,fluent-reviewers,nalexander,settings-reviewers,Gijs

When an element is re-translated, the textContent is currently dropped.
To avoid that, make it an explicit part of the message.

This requires changing the l10n message ID.
Ideally, the message ID wouldn't change, but that is not possible until
the content can be preserved as a feature, i.e.
https://github.com/projectfluent/fluent.js/issues/169

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

browser/base/content/aboutDialog-appUpdater.js

@@ -56,10 +56,13 @@ function appUpdater(options = {}) {
       Services.urlFormatter.formatURLPref("app.update.url.manual")
     );
 
-    for (const manualLink of document.getElementsByClassName("manualLink")) {
+    for (const manualLink of document.querySelectorAll(".manualLink")) {
       // Strip hash and search parameters for display text.
-      manualLink.textContent = manualURL.origin + manualURL.pathname;
+      let displayUrl = manualURL.origin + manualURL.pathname;
       manualLink.href = manualURL.href;
+      document.l10n.setArgs(manualLink.closest("[data-l10n-id]"), {
+        displayUrl,
+      });
     }
 
     document.getElementById("failedLink").href = manualURL.href;
@@ -103,7 +106,7 @@ appUpdater.prototype = {
         this.selectPanel("otherInstanceHandlingUpdates");
         break;
       case AppUpdater.STATUS.DOWNLOADING: {
-        let downloadStatus = document.getElementById("downloadStatus");
+        const downloadStatus = document.getElementById("downloading");
         if (!args.length) {
           // Very early in the DOWNLOADING state, `selectedPatch` may not be
           // available yet. But this function will be called again when it is
@@ -113,17 +116,13 @@ appUpdater.prototype = {
           if (this.update.selectedPatch) {
             maxSize = this.update.selectedPatch.size;
           }
-          downloadStatus.textContent = DownloadUtils.getTransferTotal(
-            0,
-            maxSize
-          );
+          const transfer = DownloadUtils.getTransferTotal(0, maxSize);
+          document.l10n.setArgs(downloadStatus, { transfer });
           this.selectPanel("downloading");
         } else {
           let [progress, max] = args;
-          downloadStatus.textContent = DownloadUtils.getTransferTotal(
-            progress,
-            max
-          );
+          const transfer = DownloadUtils.getTransferTotal(progress, max);
+          document.l10n.setArgs(downloadStatus, { transfer });
         }
         break;
       }

browser/base/content/aboutDialog.xhtml

@@ -77,8 +77,8 @@
                           oncommand="gAppUpdater.buttonRestartAfterDownload();"/>
                 </description>
                 <description id="checkingForUpdates" data-l10n-id="update-checkingForUpdates"/>
-                <description id="downloading" data-l10n-id="update-downloading-message">
-                  <label id="downloadStatus" data-l10n-name="download-status"/>
+                <description id="downloading" data-l10n-id="aboutdialog-update-downloading" data-l10n-args='{"transfer":""}'>
+                  <label data-l10n-name="download-status"/>
                 </description>
                 <description id="applying" data-l10n-id="update-applying"/>
                 <description id="downloadFailed" data-l10n-id="update-failed">
@@ -88,14 +88,14 @@
                 <description id="noUpdatesFound" data-l10n-id="update-noUpdatesFound"/>
                 <description id="checkingFailed" data-l10n-id="aboutdialog-update-checking-failed"/>
                 <description id="otherInstanceHandlingUpdates" data-l10n-id="update-otherInstanceHandlingUpdates"/>
-                <description id="manualUpdate" data-l10n-id="update-manual">
+                <description id="manualUpdate" data-l10n-id="aboutdialog-update-manual-with-link" data-l10n-args='{"displayUrl":""}'>
                   <label class="manualLink" is="text-link" data-l10n-name="manual-link"/>
                 </description>
                 <description id="unsupportedSystem" data-l10n-id="update-unsupported">
                   <label id="unsupportedLink" is="text-link" data-l10n-name="unsupported-link"/>
                 </description>
                 <description id="restarting" data-l10n-id="update-restarting"/>
-                <description id="internalError" data-l10n-id="update-internal-error">
+                <description id="internalError" data-l10n-id="update-internal-error2" data-l10n-args='{"displayUrl":""}'>
                   <label class="manualLink" is="text-link" data-l10n-name="manual-link"/>
                 </description>
                 <description id="noUpdater"/>

browser/components/preferences/main.inc.xhtml

@@ -547,9 +547,9 @@
                 is="highlightable-button"
                 disabled="true"/>
       </hbox>
-      <hbox id="downloading" align="start" data-l10n-id="update-downloading">
+      <hbox id="downloading" align="start" data-l10n-id="settings-update-downloading" data-l10n-args='{"transfer":""}'>
         <html:img class="update-throbber" data-l10n-name="icon"/>
-        <label id="downloadStatus" data-l10n-name="download-status"/>
+        <label data-l10n-name="download-status"/>
       </hbox>
       <hbox id="applying" align="start">
         <html:img class="update-throbber"/>
@@ -588,7 +588,7 @@
                 disabled="true"/>
       </hbox>
       <hbox id="manualUpdate" align="start">
-        <description class="face-sad" data-l10n-id="aboutdialog-update-manual">
+        <description class="face-sad" data-l10n-id="settings-update-manual-with-link" data-l10n-args='{"displayUrl":""}'>
           <html:a class="manualLink" data-l10n-name="manual-link" target="_blank"/>
         </description>
         <button data-l10n-id="update-checkForUpdatesButton"
@@ -611,7 +611,7 @@
                 disabled="true"/>
       </hbox>
       <hbox id="internalError" align="start">
-        <description class="face-sad" flex="1" data-l10n-id="update-internal-error">
+        <description class="face-sad" flex="1" data-l10n-id="update-internal-error2" data-l10n-args='{"displayUrl":""}'>
           <label class="manualLink" data-l10n-name="manual-link" is="text-link"/>
         </description>
         <button data-l10n-id="update-checkForUpdatesButton"

browser/locales/en-US/browser/aboutDialog.ftl

@@ -16,8 +16,15 @@ update-updateButton =
     .accesskey = R
 
 update-checkingForUpdates = Checking for updates…
-update-downloading = <img data-l10n-name="icon"/>Downloading update — <label data-l10n-name="download-status"/>
-update-downloading-message = Downloading update — <label data-l10n-name="download-status"/>
+
+## Variables:
+##   $transfer (string) - Transfer progress.
+
+settings-update-downloading = <img data-l10n-name="icon"/>Downloading update — <label data-l10n-name="download-status">{ $transfer }</label>
+aboutdialog-update-downloading = Downloading update — <label data-l10n-name="download-status">{ $transfer }</label>
+
+##
+
 update-applying = Applying update…
 
 update-failed = Update failed. <label data-l10n-name="failed-link">Download the latest version</label>
@@ -29,14 +36,19 @@ update-noUpdatesFound = { -brand-short-name } is up to date
 aboutdialog-update-checking-failed = Failed to check for updates.
 update-otherInstanceHandlingUpdates = { -brand-short-name } is being updated by another instance
 
-update-manual = Updates available at <label data-l10n-name="manual-link"/>
-aboutdialog-update-manual = Updates available at <a data-l10n-name="manual-link"/>
+## Variables:
+##   $displayUrl (String): URL to page with download instructions. Example: www.mozilla.org/firefox/nightly/
+
+aboutdialog-update-manual-with-link = Updates available at <label data-l10n-name="manual-link">{ $displayUrl }</label>
+settings-update-manual-with-link = Updates available at <a data-l10n-name="manual-link">{ $displayUrl }</a>
 
 update-unsupported = You can not perform further updates on this system. <label data-l10n-name="unsupported-link">Learn more</label>
 
 update-restarting = Restarting…
 
-update-internal-error = Unable to check for updates due to internal error. Updates available at <label data-l10n-name="manual-link"/>
+update-internal-error2 = Unable to check for updates due to internal error. Updates available at <label data-l10n-name="manual-link">{ $displayUrl }</label>
+
+##
 
 # Variables:
 #   $channel (String): description of the update channel (e.g. "release", "beta", "nightly" etc.)

python/l10n/fluent_migrations/bug_1835559_aboutDialog_explicit_textContent.py

+# Any copyright is dedicated to the Public Domain.
+# http://creativecommons.org/publicdomain/zero/1.0/
+
+import fluent.syntax.ast as FTL
+
+from fluent.migrate.transforms import TransformPattern
+
+
+class INSERT_VAR_IN_TAG(TransformPattern):
+    def visit_TextElement(self, node):
+        old_str = node.value
+        new_str = old_str
+        # update-downloading / update-downloading-message
+        new_str = new_str.replace(
+            '<label data-l10n-name="download-status"/>',
+            '<label data-l10n-name="download-status">{ $transfer }</label>',
+        )
+        # update-manual / update-internal-error
+        new_str = new_str.replace(
+            '<label data-l10n-name="manual-link"/>',
+            '<label data-l10n-name="manual-link">{ $displayUrl }</label>',
+        )
+        # aboutdialog-update-manual
+        new_str = new_str.replace(
+            '<a data-l10n-name="manual-link"/>',
+            '<a data-l10n-name="manual-link">{ $displayUrl }</a>',
+        )
+        if old_str == new_str and "$" not in old_str:
+            print("Warning: Failed to insert var in link in {}".format(old_str))
+        else:
+            node.value = new_str
+        return node
+
+
+def migrate(ctx):
+    """Bug 1835559, insert textContent var in a/label elements, part {index}."""
+
+    aboutDialog_ftl = "browser/browser/aboutDialog.ftl"
+    ctx.add_transforms(
+        aboutDialog_ftl,
+        aboutDialog_ftl,
+        [
+            FTL.Message(
+                id=FTL.Identifier("settings-update-downloading"),
+                value=INSERT_VAR_IN_TAG(aboutDialog_ftl, "update-downloading"),
+            ),
+            FTL.Message(
+                id=FTL.Identifier("aboutdialog-update-downloading"),
+                value=INSERT_VAR_IN_TAG(aboutDialog_ftl, "update-downloading-message"),
+            ),
+            # Note: while we're renaming anyway: strip "aboutdialog-" prefix
+            # because it is used in main.inc.xhtml, not aboutDialog.xhtml.
+            FTL.Message(
+                id=FTL.Identifier("settings-update-manual-with-link"),
+                value=INSERT_VAR_IN_TAG(aboutDialog_ftl, "aboutdialog-update-manual"),
+            ),
+            # This is the actual update-manual message in aboutDialog.xhtml.
+            FTL.Message(
+                id=FTL.Identifier("aboutdialog-update-manual-with-link"),
+                value=INSERT_VAR_IN_TAG(aboutDialog_ftl, "update-manual"),
+            ),
+            FTL.Message(
+                id=FTL.Identifier("update-internal-error2"),
+                value=INSERT_VAR_IN_TAG(aboutDialog_ftl, "update-internal-error"),
+            ),
+        ],
+    )

toolkit/mozapps/update/tests/browser/head.js

@@ -805,7 +805,9 @@ function runAboutDialogUpdateTest(params, steps) {
               "Sanity check: Expected download status text should be non-empty"
             );
             Assert.equal(
-              aboutDialog.document.getElementById("downloadStatus").textContent,
+              aboutDialog.document.querySelector(
+                `#downloading label[data-l10n-name="download-status"]`
+              ).textContent,
               expectedText,
               "Download status text should be correct"
             );
@@ -825,7 +827,14 @@ function runAboutDialogUpdateTest(params, steps) {
         // The unsupportedSystem panel uses the update's detailsURL and the
         // downloadFailed and manualUpdate panels use the app.update.url.manual
         // preference.
-        let link = selectedPanel.querySelector("label.text-link");
+        let selector = "label.text-link";
+        if (selectedPanel.ownerDocument.hasPendingL10nMutations) {
+          await BrowserTestUtils.waitForEvent(
+            selectedPanel.ownerDocument,
+            "L10nMutationsFinished"
+          );
+        }
+        let link = selectedPanel.querySelector(selector);
         is(
           link.href,
           gDetailsURL,
@@ -835,16 +844,12 @@ function runAboutDialogUpdateTest(params, steps) {
           let textContent = node.textContent.trim();
           ok(textContent, `${description}, got "${textContent}"`);
         };
-        // TODO bug 1835559: Somehow the text content is empty.
-        if (panelId !== "internalError") {
-          assertNonEmptyText(
-            link,
-            `The panel's link should have non-empty textContent`
-          );
-        }
+        assertNonEmptyText(
+          link,
+          `The panel's link should have non-empty textContent`
+        );
         let linkWrapperClone = link.parentNode.cloneNode(true);
-        await link.ownerDocument.l10n.translateFragment(linkWrapperClone);
-        linkWrapperClone.querySelector("label.text-link").remove();
+        linkWrapperClone.querySelector(selector).remove();
         assertNonEmptyText(
           linkWrapperClone,
           `The panel's link should have text around the link`
@@ -1146,6 +1151,12 @@ function runAboutPrefsUpdateTest(params, steps) {
             if (selectedPanel.id == "manualUpdate") {
               selector = "a.manualLink";
             }
+            if (selectedPanel.ownerDocument.hasPendingL10nMutations) {
+              await BrowserTestUtils.waitForEvent(
+                selectedPanel.ownerDocument,
+                "L10nMutationsFinished"
+              );
+            }
             let link = selectedPanel.querySelector(selector);
             is(
               link.href,
@@ -1156,15 +1167,11 @@ function runAboutPrefsUpdateTest(params, steps) {
               let textContent = node.textContent.trim();
               ok(textContent, `${description}, got "${textContent}"`);
             };
-            // TODO bug 1835559: Somehow the text content is empty.
-            if (panelId !== "internalError") {
-              assertNonEmptyText(
-                link,
-                `The panel's link should have non-empty textContent`
-              );
-            }
+            assertNonEmptyText(
+              link,
+              `The panel's link should have non-empty textContent`
+            );
             let linkWrapperClone = link.parentNode.cloneNode(true);
-            await link.ownerDocument.l10n.translateFragment(linkWrapperClone);
             linkWrapperClone.querySelector(selector).remove();
             assertNonEmptyText(
               linkWrapperClone,