Make sure translations can only use two HTML tags without any attributes - no custom HTML code in t…

chrome/content/ui/i18n.js

Index: chrome/content/ui/i18n.js
===================================================================
--- a/chrome/content/ui/i18n.js
+++ b/chrome/content/ui/i18n.js
@@ -18,29 +18,29 @@
 var i18n;
 if (typeof chrome != "undefined")
 {
   i18n = chrome.i18n;
 }
 else
 {
   // Using Firefox' approach on i18n instead
-  
+
   // Randomize URI to work around bug 719376
   var pageName = location.pathname.replace(/.*\//, '').replace(/\..*?$/, '');
   var stringBundle = Services.strings.createBundle("chrome://adblockplus/locale/" + pageName +
     ".properties?" + Math.random());
-  
+
   function getI18nMessage(key)
   {
     return {
       "message": stringBundle.GetStringFromName(key)
     };
   }
-  
+
   i18n = (function()
   {
     function getText(message, args)
     {
       var text = message.message;
       var placeholders = message.placeholders;
 
       if (!args || !placeholders)
@@ -82,29 +82,49 @@ else
   })();
 }
 
 // Loads and inserts i18n strings into matching elements. Any inner HTML already in the
 // element is parsed as JSON and used as parameters to substitute into placeholders in the
 // i18n message.
 function loadI18nStrings()
 {
+  function processString(str, element)
+  {
+    var match = /^(.*?)<(a|strong)>(.*?)<\/\2>(.*)$/.exec(str);
+    if (match)
+    {
+      processString(match[1], element);
+
+      var e = document.createElement(match[2]);
+      processString(match[3], e);
+      element.appendChild(e);
+
+      processString(match[4], element);
+    }
+    else
+      element.appendChild(document.createTextNode(str));
+  }
+
   var nodes = document.querySelectorAll("[class^='i18n_']");
   for(var i = 0; i < nodes.length; i++)
   {
-    var arguments = JSON.parse("[" + nodes[i].textContent + "]");
-    var className = nodes[i].className;
+    var node = nodes[i];
+    var arguments = JSON.parse("[" + node.textContent + "]");
+    if (arguments.length == 0)
+      arguments = null;
+
+    var className = node.className;
     if (className instanceof SVGAnimatedString)
       className = className.animVal;
     var stringName = className.split(/\s/)[0].substring(5);
-    var prop = "innerHTML" in nodes[i] ? "innerHTML" : "textContent";
-    if(arguments.length > 0)
-      nodes[i][prop] = i18n.getMessage(stringName, arguments);
-    else
-      nodes[i][prop] = i18n.getMessage(stringName);
+
+    while (node.lastChild)
+      node.removeChild(node.lastChild);

[Thomas Greiner, 2013/07/10 08:45:38]

node.innerHTML = "";
is increasingly faster the more child nodes a node has but given that those
nodes usually don't have more than one child node it shouldn't matter which
approach we use here

[Wladimir Palant, 2013/07/10 09:07:23]

I would rather avoid using innerHTML for anything - seeing it used in an
extension is usually a bad sign. In particular, AMO's extension validator will
show a warning if it finds innerHTML somewhere, the reviewers then need to
verify manually that its use if benign.

[Thomas Greiner, 2013/07/10 09:41:43]

I wasn't aware of that review process. I assume that's not the case for
textContent because it has the same effect if you do
node.textContent = "";

+    processString(i18n.getMessage(stringName, arguments), node);
   }
 }
 
 // Provides a more readable string of the current date and time
 function i18n_timeDateStrings(when)
 {
   var d = new Date(when);
   var timeString = d.toLocaleTimeString();