Bug 1431428 - use DOM instead of innerHTML for extension messaging in prefs, r?jaws draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 18 Jan 2018 18:19:10 +0000
changeset 723445 44711904a2da6a0d499d6b077f21d68ae4e1cf73
parent 723364 c4ebc8c28a33b785dfbfa533810517cc707d1ad0
child 746864 1f3316280d0ec0f04b0ff1d45a07a77f075d0c4a
push id96434
push usergijskruitbosch@gmail.com
push dateTue, 23 Jan 2018 10:35:55 +0000
reviewersjaws
bugs1431428
milestone60.0a1
Bug 1431428 - use DOM instead of innerHTML for extension messaging in prefs, r?jaws MozReview-Commit-ID: JkG6zT8ADDU
browser/base/content/browser-media.js
browser/components/preferences/in-content/findInPage.js
browser/components/preferences/in-content/preferences.js
browser/locales/en-US/chrome/browser/preferences/preferences.properties
toolkit/modules/BrowserUtils.jsm
--- a/browser/base/content/browser-media.js
+++ b/browser/base/content/browser-media.js
@@ -31,27 +31,23 @@ var gEMEHandler = {
     if (keySystem == "com.widevine.alpha" &&
         Services.prefs.getPrefType("media.gmp-widevinecdm.visible")) {
       return Services.prefs.getBoolPref("media.gmp-widevinecdm.visible");
     }
     return true;
   },
   getEMEDisabledFragment(msgId) {
     let mainMessage = gNavigatorBundle.getString("emeNotifications.drmContentDisabled.message");
-    let [prefix, suffix] = mainMessage.split(/%(?:1\$)?S/).map(s => document.createTextNode(s));
     let text = gNavigatorBundle.getString("emeNotifications.drmContentDisabled.learnMoreLabel");
     let baseURL = Services.urlFormatter.formatURLPref("app.support.baseURL");
     let link = document.createElement("label");
     link.className = "text-link";
     link.setAttribute("href", baseURL + "drm-content");
     link.textContent = text;
-
-    let fragment = document.createDocumentFragment();
-    [prefix, link, suffix].forEach(n => fragment.appendChild(n));
-    return fragment;
+    return BrowserUtils.getLocalizedFragment(document, mainMessage, link);
   },
   getMessageWithBrandName(notificationId) {
     let msgId = "emeNotifications." + notificationId + ".message";
     return gNavigatorBundle.getFormattedString(msgId, [this._brandShortName]);
   },
   receiveMessage({target: browser, data: data}) {
     let parsedData;
     try {
--- a/browser/components/preferences/in-content/findInPage.js
+++ b/browser/components/preferences/in-content/findInPage.js
@@ -298,28 +298,25 @@ var gSearchResultsPane = {
         let strings = this.strings;
 
         document.getElementById("sorry-message").textContent = AppConstants.platform == "win" ?
           strings.getFormattedString("searchResults.sorryMessageWin", [this.query]) :
           strings.getFormattedString("searchResults.sorryMessageUnix", [this.query]);
         let helpUrl = Services.urlFormatter.formatURLPref("app.support.baseURL") + "preferences";
         let brandName = document.getElementById("bundleBrand").getString("brandShortName");
         let helpString = strings.getString("searchResults.needHelp3");
-        let helpItems = helpString.split(/%(?:1\$)?S/);
         let helpContainer = document.getElementById("need-help");
-        helpContainer.innerHTML = "";
-        helpContainer.appendChild(document.createTextNode(helpItems[0]));
         let link = document.createElement("label");
         link.className = "text-link";
         link.setAttribute("href", helpUrl);
         link.textContent = strings.getFormattedString("searchResults.needHelpSupportLink", [brandName]);
-        helpContainer.appendChild(link);
-        if (helpItems[1]) {
-          helpContainer.appendChild(document.createTextNode(helpItems[1]));
-        }
+
+        helpContainer.innerHTML = "";
+        let fragment = BrowserUtils.getLocalizedFragment(document, helpString, link);
+        helpContainer.appendChild(fragment);
       } else {
         // Creating tooltips for all the instances found
         for (let anchorNode of this.listSearchTooltips) {
           this.createSearchTooltip(anchorNode, this.query);
         }
 
         // Implant search telemetry probe after user stops typing for a while
         if (this.query.length >= 2) {
--- a/browser/components/preferences/in-content/preferences.js
+++ b/browser/components/preferences/in-content/preferences.js
@@ -19,20 +19,22 @@ var Cc = Components.classes;
 var Ci = Components.interfaces;
 var Cu = Components.utils;
 var Cr = Components.results;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/AppConstants.jsm");
 
+XPCOMUtils.defineLazyModuleGetter(this, "AddonManager",
+                                  "resource://gre/modules/AddonManager.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "BrowserUtils",
+                                  "resource://gre/modules/BrowserUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionSettingsStore",
                                   "resource://gre/modules/ExtensionSettingsStore.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "AddonManager",
-                                  "resource://gre/modules/AddonManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "formAutofillParent",
                                   "resource://formautofill/FormAutofillParent.jsm");
 
 XPCOMUtils.defineLazyPreferenceGetter(this, "trackingprotectionUiEnabled",
                                       "privacy.trackingprotection.ui.enabled");
 
 var gLastHash = "";
 
@@ -490,36 +492,35 @@ async function handleControllingExtensio
   return !!addon;
 }
 
 async function showControllingExtension(settingName, addon) {
   // Tell the user what extension is controlling the setting.
   let elements = getControllingExtensionEls(settingName);
 
   elements.section.classList.remove("extension-controlled-disabled");
-  const defaultIcon = "chrome://mozapps/skin/extensions/extensionGeneric.svg";
-  let stringParts = document
-    .getElementById("bundlePreferences")
-    .getString(`extensionControlled.${settingName}`)
-    .split("%S");
   let description = elements.description;
 
   // Remove the old content from the description.
   while (description.firstChild) {
     description.firstChild.remove();
   }
 
   // Populate the description.
-  description.appendChild(document.createTextNode(stringParts[0]));
+  let msg = document.getElementById("bundlePreferences")
+                    .getString(`extensionControlled.${settingName}`);
   let image = document.createElement("image");
+  const defaultIcon = "chrome://mozapps/skin/extensions/extensionGeneric.svg";
   image.setAttribute("src", addon.iconURL || defaultIcon);
   image.classList.add("extension-controlled-icon");
-  description.appendChild(image);
-  description.appendChild(document.createTextNode(` ${addon.name}`));
-  description.appendChild(document.createTextNode(stringParts[1]));
+  let addonBit = document.createDocumentFragment();
+  addonBit.appendChild(image);
+  addonBit.appendChild(document.createTextNode(" " + addon.name));
+  let fragment = BrowserUtils.getLocalizedFragment(document, msg, addonBit);
+  description.appendChild(fragment);
 
   if (elements.button) {
     elements.button.hidden = false;
   }
 
   // Show the controlling extension row and hide the old label.
   elements.section.hidden = false;
 }
@@ -532,24 +533,29 @@ function hideControllingExtension(settin
   }
 }
 
 function showEnableExtensionMessage(settingName) {
   let elements = getControllingExtensionEls(settingName);
 
   elements.button.hidden = true;
   elements.section.classList.add("extension-controlled-disabled");
-  let icon = url => `<image src="${url}" class="extension-controlled-icon"/>`;
+  let icon = url => {
+    let img = document.createElement("image");
+    img.src = url;
+    img.className = "extension-controlled-icon";
+    return img;
+  };
   let addonIcon = icon("chrome://mozapps/skin/extensions/extensionGeneric-16.svg");
   let toolbarIcon = icon("chrome://browser/skin/menu.svg");
-  let message = document
-    .getElementById("bundlePreferences")
-    .getFormattedString("extensionControlled.enable", [addonIcon, toolbarIcon]);
-  // eslint-disable-next-line no-unsanitized/property
-  elements.description.innerHTML = message;
+  let message = document.getElementById("bundlePreferences")
+                        .getString("extensionControlled.enable2");
+  let frag = BrowserUtils.getLocalizedFragment(document, message, addonIcon, toolbarIcon);
+  elements.description.innerHTML = "";
+  elements.description.appendChild(frag);
   let dismissButton = document.createElement("image");
   dismissButton.setAttribute("class", "extension-controlled-icon close-icon");
   dismissButton.addEventListener("click", function dismissHandler() {
     hideControllingExtension(settingName);
     dismissButton.removeEventListener("click", dismissHandler);
   });
   elements.description.appendChild(dismissButton);
 }
--- a/browser/locales/en-US/chrome/browser/preferences/preferences.properties
+++ b/browser/locales/en-US/chrome/browser/preferences/preferences.properties
@@ -297,10 +297,9 @@ extensionControlled.privacy.containers =
 # LOCALIZATION NOTE (extensionControlled.websites.trackingProtectionMode):
 # This string is shown to notify the user that their tracking protection preferences are being controlled by an extension.
 extensionControlled.websites.trackingProtectionMode = An extension, %S, is controlling tracking protection.
 
 # LOCALIZATION NOTE (extensionControlled.enable):
 # %1$S is replaced with the icon for the add-ons menu.
 # %2$S is replaced with the icon for the toolbar menu.
 # This string is shown to notify the user how to enable an extension that they disabled.
-# Note, this string will be used as raw markup. Avoid characters like <, >, &
-extensionControlled.enable = To enable the extension go to %1$S Add-ons in the %2$S menu.
+extensionControlled.enable2 = To enable the extension go to %1$S Add-ons in the %2$S menu.
--- a/toolkit/modules/BrowserUtils.jsm
+++ b/toolkit/modules/BrowserUtils.jsm
@@ -718,9 +718,75 @@ this.BrowserUtils = {
                    .getInterface(Ci.nsIDOMWindowUtils);
 
     if (!utils.needsFlush(FLUSH_TYPES[flushType])) {
       return callback();
     }
 
     return this.promiseReflowed(doc, callback);
   },
+
+  /**
+   * Generate a document fragment for a localized string that has DOM
+   * node replacements. This avoids using getFormattedString followed
+   * by assigning to innerHTML. Fluent can probably replace this when
+   * it is in use everywhere.
+   *
+   * @param {Document} doc
+   * @param {String}   msg
+   *                   The string to put replacements in. Fetch from
+   *                   a stringbundle using getString or GetStringFromName,
+   *                   or even an inserted dtd string.
+   * @param {Node|String} nodesOrStrings
+   *                   The replacement items. Can be a mix of Nodes
+   *                   and Strings. However, for correct behaviour, the
+   *                   number of items provided needs to exactly match
+   *                   the number of replacement strings in the l10n string.
+   * @returns {DocumentFragment}
+   *                   A document fragment. In the trivial case (no
+   *                   replacements), this will simply be a fragment with 1
+   *                   child, a text node containing the localized string.
+   */
+  getLocalizedFragment(doc, msg, ...nodesOrStrings) {
+    // Ensure replacement points are indexed:
+    for (let i = 1; i <= nodesOrStrings.length; i++) {
+      if (!msg.includes("%" + i + "$S")) {
+        msg = msg.replace(/%S/, "%" + i + "$S");
+      }
+    }
+    let numberOfInsertionPoints = msg.match(/%\d+\$S/g).length;
+    if (numberOfInsertionPoints != nodesOrStrings.length) {
+      Cu.reportError(`Message has ${numberOfInsertionPoints} insertion points, ` +
+                     `but got ${nodesOrStrings.length} replacement parameters!`);
+    }
+
+    let fragment = doc.createDocumentFragment();
+    let parts = [msg];
+    let insertionPoint = 1;
+    for (let replacement of nodesOrStrings) {
+      let insertionString = "%" + (insertionPoint++) + "$S";
+      let partIndex = parts.findIndex(part => typeof part == "string" && part.includes(insertionString));
+      if (partIndex == -1) {
+        fragment.appendChild(doc.createTextNode(msg));
+        return fragment;
+      }
+
+      if (typeof replacement == "string") {
+        parts[partIndex] = parts[partIndex].replace(insertionString, replacement);
+      } else {
+        let [firstBit, lastBit] = parts[partIndex].split(insertionString);
+        parts.splice(partIndex, 1, firstBit, replacement, lastBit);
+      }
+    }
+
+    // Put everything in a document fragment:
+    for (let part of parts) {
+      if (typeof part == "string") {
+        if (part) {
+          fragment.appendChild(doc.createTextNode(part));
+        }
+      } else {
+        fragment.appendChild(part);
+      }
+    }
+    return fragment;
+  },
 };