Bug 1012530 - Part 1. Keep information about placeholder nodes. r=florian
authorFelipe Gomes <felipc@gmail.com>
Sun, 08 Jun 2014 13:02:13 -0300
changeset 207808 f0f129affe2fa2d14ed7d8e7b59c8baaccef098f
parent 207807 0f82d42806cb4e7eb542ce4f1ab58171ad91d8dc
child 207809 09f8e14a234968c1273a46f222d9c1831de88279
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersflorian
bugs1012530
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1012530 - Part 1. Keep information about placeholder nodes. r=florian
browser/components/translation/TranslationDocument.jsm
--- a/browser/components/translation/TranslationDocument.jsm
+++ b/browser/components/translation/TranslationDocument.jsm
@@ -132,42 +132,53 @@ this.TranslationDocument.prototype = {
     if (item.isSimpleRoot) {
       let text = item.nodeRef.firstChild.nodeValue.trim();
       item.original = [text];
       return text;
     }
 
     let str = "";
     item.original = [];
+    let wasLastItemPlaceholder = false;
 
     for (let child of item.nodeRef.childNodes) {
       if (child.nodeType == TEXT_NODE) {
         let x = child.nodeValue.trim();
-        str += x;
-        item.original.push(x);
+        if (x != "") {
+          item.original.push(x);
+          str += x;
+          wasLastItemPlaceholder = false;
+        }
         continue;
       }
 
       let objInMap = this.itemsMap.get(child);
       if (objInMap && !objInMap.isRoot) {
         // If this childNode is present in the itemsMap, it means
         // it's a translation node: it has useful content for translation.
         // In this case, we need to stringify this node.
         // However, if this item is a root, we should skip it here in this
         // object's child list (and just add a placeholder for it), because
         // it will be stringfied separately for being a root.
         item.original.push(objInMap);
         str += this.generateTextForItem(objInMap);
+        wasLastItemPlaceholder = false;
       } else {
         // Otherwise, if this node doesn't contain any useful content,
         // or if it is a root itself, we can replace it with a placeholder node.
         // We can't simply eliminate this node from our string representation
         // because that could change the HTML structure (e.g., it would
         // probably merge two separate text nodes).
-        str += '<br/>';
+        // It's not necessary to add more than one placeholder in sequence;
+        // we can optimize them away.
+        if (!wasLastItemPlaceholder) {
+          item.original.push(TranslationItem_NodePlaceholder);
+          str += '<br>';
+          wasLastItemPlaceholder = true;
+        }
       }
     }
 
     return generateTranslationHtmlForItem(item, str);
   },
 
   /**
    * Changes the document to display its translated
@@ -254,18 +265,18 @@ function TranslationItem(node, id, isRoo
   this.children = [];
 }
 
 TranslationItem.prototype = {
   isRoot: false,
   isSimpleRoot: false,
 
   toString: function() {
-    let rootType = this._isRoot
-                   ? (this._isSimpleRoot ? ' (simple root)' : ' (non simple root)')
+    let rootType = this.isRoot
+                   ? (this.isSimpleRoot ? ' (simple root)' : ' (non simple root)')
                    : '';
     return "[object TranslationItem: <" + this.nodeRef.localName + ">"
            + rootType + "]";
   },
 
   /**
    * This function will parse the result of the translation of one translation
    * item. If this item was a simple root, all we sent was a plain-text version
@@ -320,16 +331,29 @@ TranslationItem.prototype = {
    *                 or "original".
    */
   swapText: function(target) {
     swapTextForItem(this, target);
   }
 };
 
 /**
+ * This object represents a placeholder item for translation. It's similar to
+ * the TranslationItem class, but it represents nodes that have no meaningful
+ * content for translation. These nodes will be replaced by "<br>" in a
+ * translation request. It's necessary to keep them to use it as a mark
+ * for correct positioning and spliting of text nodes.
+ */
+const TranslationItem_NodePlaceholder = {
+  toString: function() {
+    return "[object TranslationItem_NodePlaceholder]";
+  }
+};
+
+/**
  * Generate the outer HTML representation for a given item.
  *
  * @param   item       A TranslationItem object.
  * param    content    The inner content for this item.
  * @returns string     The outer HTML needed for translation
  *                     of this item.
  */
 function generateTranslationHtmlForItem(item, content) {
@@ -349,32 +373,23 @@ function generateTranslationHtmlForItem(
  * @returns        A string representation of the TranslationItem.
  */
 function regenerateTextFromOriginalHelper(item) {
   if (item.isSimpleRoot) {
     return item.original[0];
   }
 
   let str = "";
-
-  let wasLastItemText = false;
   for (let child of item.original) {
     if (child instanceof TranslationItem) {
       str += regenerateTextFromOriginalHelper(child);
-      wasLastItemText = false;
+    } else if (child === TranslationItem_NodePlaceholder) {
+      str += "<br>";
     } else {
-      // The non-significant elements (which were replaced with <br/>
-      // during the first call to generateTextForItem) are not stored
-      // in the original array. If they are not properly re-generated,
-      // two adjacent text nodes would be merged into one.
-      if (wasLastItemText) {
-        str += "<br/>";
-      }
       str += child;
-      wasLastItemText = true;
     }
   }
 
   return generateTranslationHtmlForItem(item, str);
 }
 
 /**
  * Helper function to parse a HTML doc result.
@@ -390,16 +405,18 @@ function regenerateTextFromOriginalHelpe
  *
  * For text nodes we simply add it as a string.
  */
 function parseResultNode(item, node) {
   item.translation = [];
   for (let child of node.childNodes) {
     if (child.nodeType == TEXT_NODE) {
       item.translation.push(child.nodeValue);
+    } else if (child.localName == "br") {
+      item.translation.push(TranslationItem_NodePlaceholder);
     } else {
       let translationItemChild = item.getChildById(child.id);
 
       if (translationItemChild) {
         item.translation.push(translationItemChild);
         parseResultNode(translationItemChild, child);
       }
     }