From 04350197cb063c1061dafa845119cc19fc2690b4 Mon Sep 17 00:00:00 2001 From: Felipe Gomes Date: Fri, 23 May 2014 13:08:18 -0300 Subject: [PATCH] Bug 1012519 - Re-translation should use original content instead of newly translated content. r=florian --- .../translation/TranslationContentHandler.jsm | 7 +- .../translation/TranslationDocument.jsm | 67 +++++++++++++++++-- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/browser/components/translation/TranslationContentHandler.jsm b/browser/components/translation/TranslationContentHandler.jsm index efbb3d87ada0..8ff40158f21c 100644 --- a/browser/components/translation/TranslationContentHandler.jsm +++ b/browser/components/translation/TranslationContentHandler.jsm @@ -67,7 +67,12 @@ TranslationContentHandler.prototype = { Cu.import("resource:///modules/translation/TranslationDocument.jsm"); Cu.import("resource:///modules/translation/BingTranslator.jsm"); - let translationDocument = new TranslationDocument(this.global.content.document); + // If a TranslationDocument already exists for this document, it should + // be used instead of creating a new one so that we can use the original + // content of the page for the new translation instead of the newly + // translated text. + let translationDocument = this.global.content.translationDocument || + new TranslationDocument(this.global.content.document); let bingTranslation = new BingTranslation(translationDocument, msg.data.from, msg.data.to); diff --git a/browser/components/translation/TranslationDocument.jsm b/browser/components/translation/TranslationDocument.jsm index 5be94c4cbc1e..f424959666bf 100644 --- a/browser/components/translation/TranslationDocument.jsm +++ b/browser/components/translation/TranslationDocument.jsm @@ -115,21 +115,27 @@ this.TranslationDocument.prototype = { * Besides generating the string, it's also stored in the "original" * field of the TranslationItem object, which needs to be stored for * later to be used in the "Show Original" functionality. + * If this function had already been called for the given item (determined + * by the presence of the "original" array in the item), the text will + * be regenerated from the "original" data instead of from the related + * DOM nodes (because the nodes might contain translated data). * * @param item A TranslationItem object * * @returns A string representation of the TranslationItem. */ generateTextForItem: function(item) { + if (item.original) { + return regenerateTextFromOriginalHelper(item); + } + if (item.isSimpleRoot) { let text = item.nodeRef.firstChild.nodeValue.trim(); item.original = [text]; return text; } - let localName = item.isRoot ? "div" : "b"; - let str = '<' + localName + ' id="n' + item.id + '">'; - + let str = ""; item.original = []; for (let child of item.nodeRef.childNodes) { @@ -157,8 +163,7 @@ this.TranslationDocument.prototype = { } } - str += ''; - return str; + return generateTranslationHtmlForItem(item, str); }, /** @@ -316,6 +321,58 @@ TranslationItem.prototype = { } }; +/** + * 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) { + let localName = item.isRoot ? "div" : "b"; + return '<' + localName + ' id="n' + item.id + '">' + + content + + ""; +} + + /** + * Regenerate the text string that represents a TranslationItem object, + * with data from its "original" array. The array must have already + * been created by TranslationDocument.generateTextForItem(). + * + * @param item A TranslationItem object + * + * @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 { + // The non-significant elements (which were replaced with
+ // 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 += "
"; + } + str += child; + wasLastItemText = true; + } + } + + return generateTranslationHtmlForItem(item, str); +} + /** * Helper function to parse a HTML doc result. * How it works: