Bug 1363862 - Use Node.localize for fragment translation in Fluent DOM. r?stas
MozReview-Commit-ID: 1zAbSMapi86
--- a/browser/components/preferences/in-content/tests/browser_fluent.js
+++ b/browser/components/preferences/in-content/tests/browser_fluent.js
@@ -36,14 +36,14 @@ add_task(async function() {
]);
let elem = doc.querySelector(
`#contentProcessCount > menupopup > menuitem[value="${defaultProcessCount}"]`);
Assert.deepEqual(msg, {
value: null,
attrs: [
- ["label", elem.getAttribute("label")]
+ {name: "label", value: elem.getAttribute("label")}
]
});
await BrowserTestUtils.removeTab(gBrowser.selectedTab);
});
--- a/intl/l10n/DOMLocalization.jsm
+++ b/intl/l10n/DOMLocalization.jsm
@@ -102,19 +102,19 @@ function overlayElement(targetElement, t
// translation.
for (const attr of Array.from(targetElement.attributes)) {
if (isAttrNameLocalizable(attr.name, targetElement, explicitlyAllowed)) {
targetElement.removeAttribute(attr.name);
}
}
if (translation.attrs) {
- for (const [name, val] of translation.attrs) {
+ for (const {name, value} of translation.attrs) {
if (isAttrNameLocalizable(name, targetElement, explicitlyAllowed)) {
- targetElement.setAttribute(name, val);
+ targetElement.setAttribute(name, value);
}
}
}
}
/**
* Sanitize `translationFragment` using `sourceElement` to add functional
* HTML attributes to children. `sourceElement` will have all its child nodes
@@ -299,16 +299,57 @@ function shiftNamedElement(element, loca
if (child.localName === localName) {
element.removeChild(child);
return child;
}
}
return null;
}
+/**
+ * Sanitizes a translation before passing them to Node.localize API.
+ *
+ * It returns `false` if the translation contains DOM Overlays and should
+ * not go into Node.localize.
+ *
+ * Note: There's a third item of work that JS DOM Overlays do - removal
+ * of attributes from the previous translation.
+ * This is not trivial to implement for Node.localize scenario, so
+ * at the moment it is not supported.
+ *
+ * @param {{
+ * localName: string,
+ * namespaceURI: string,
+ * type: string || null
+ * l10nId: string,
+ * l10nArgs: Array<Object> || null,
+ * l10nAttrs: string ||null,
+ * }} l10nItems
+ * @param {{value: string, attrs: Object}} translations
+ * @returns boolean
+ * @private
+ */
+function sanitizeTranslationForNodeLocalize(l10nItem, translation) {
+ if (reOverlay.test(translation.value)) {
+ return false;
+ }
+
+ if (translation.attrs) {
+ const explicitlyAllowed = l10nItem.l10nAttrs === null ? null :
+ l10nItem.l10nAttrs.split(",").map(i => i.trim());
+ for (const [j, {name}] of translation.attrs.entries()) {
+ if (!isAttrNameLocalizable(name, l10nItem, explicitlyAllowed)) {
+ translation.attrs.splice(j, 1);
+ }
+ }
+ }
+ return true;
+}
+
+
const L10NID_ATTR_NAME = "data-l10n-id";
const L10NARGS_ATTR_NAME = "data-l10n-args";
const L10N_ELEMENT_QUERY = `[${L10NID_ATTR_NAME}]`;
/**
* The `DOMLocalization` class is responsible for fetching resources and
* formatting translations.
@@ -463,17 +504,17 @@ class DOMLocalization extends Localizati
/**
* Translate all roots associated with this `DOMLocalization`.
*
* @returns {Promise}
*/
translateRoots() {
const roots = Array.from(this.roots);
return Promise.all(
- roots.map(root => this.translateElements(this.getTranslatables(root)))
+ roots.map(root => this.translateFragment(root))
);
}
/**
* Pauses the `MutationObserver`.
*
* @private
*/
@@ -528,31 +569,83 @@ class DOMLocalization extends Localizati
this.translateElements(Array.from(this.pendingElements));
this.pendingElements.clear();
this.pendingrAF = null;
});
}
}
}
-
/**
* Translate a DOM element or fragment asynchronously using this
* `DOMLocalization` object.
*
* Manually trigger the translation (or re-translation) of a DOM fragment.
* Use the `data-l10n-id` and `data-l10n-args` attributes to mark up the DOM
* with information about which translations to use.
*
* Returns a `Promise` that gets resolved once the translation is complete.
*
* @param {DOMFragment} frag - Element or DocumentFragment to be translated
* @returns {Promise}
*/
translateFragment(frag) {
+ if (frag.localize) {
+ // This is a temporary fast-path offered by Gecko to workaround performance
+ // issues coming from Fluent and XBL+Stylo performing unnecesary
+ // operations during startup.
+ // For details see bug 1441037, bug 1442262, and bug 1363862.
+
+ // A sparse array which will store translations separated out from
+ // all translations that is needed for DOM Overlay.
+ const overlayTranslations = [];
+
+ const getTranslationsForItems = async l10nItems => {
+ const keys = l10nItems.map(l10nItem => [l10nItem.l10nId, l10nItem.l10nArgs]);
+ const translations = await this.formatMessages(keys);
+
+ // Here we want to separate out elements that require DOM Overlays.
+ // Those elements will have to be translated using our JS
+ // implementation, while everything else is going to use the fast-path.
+ for (const [i, translation] of translations.entries()) {
+ if (translation === undefined) {
+ continue;
+ }
+
+ const hasOnlyText =
+ sanitizeTranslationForNodeLocalize(l10nItems[i], translation);
+ if (!hasOnlyText) {
+ // Removing from translations to make Node.localize skip it.
+ // We will translate it below using JS DOM Overlays.
+ overlayTranslations[i] = translations[i];
+ translations[i] = undefined;
+ }
+ }
+
+ // We pause translation observing here because Node.localize
+ // will translate the whole DOM next, using the `translations`.
+ //
+ // The observer will be resumed after DOM Overlays are localized
+ // in the next microtask.
+ this.pauseObserving();
+ return translations;
+ };
+
+ return frag.localize(getTranslationsForItems.bind(this))
+ .then(untranslatedElements => {
+ for (let i = 0; i < overlayTranslations.length; i++) {
+ if (overlayTranslations[i] !== undefined &&
+ untranslatedElements[i] !== undefined) {
+ overlayElement(untranslatedElements[i], overlayTranslations[i]);
+ }
+ }
+ this.resumeObserving();
+ })
+ .catch(() => this.resumeObserving());
+ }
return this.translateElements(this.getTranslatables(frag));
}
/**
* Translate a list of DOM elements asynchronously using this
* `DOMLocalization` object.
*
* Manually trigger the translation (or re-translation) of a list of elements.
@@ -580,17 +673,19 @@ class DOMLocalization extends Localizati
* @param {Array<Element>} elements
* @param {Array<Object>} translations
* @private
*/
applyTranslations(elements, translations) {
this.pauseObserving();
for (let i = 0; i < elements.length; i++) {
- overlayElement(elements[i], translations[i]);
+ if (translations[i] !== undefined) {
+ overlayElement(elements[i], translations[i]);
+ }
}
this.resumeObserving();
}
/**
* Collects all translatable child elements of the element.
*
--- a/intl/l10n/Localization.jsm
+++ b/intl/l10n/Localization.jsm
@@ -352,23 +352,20 @@ function messageFromContext(ctx, errors,
const formatted = {
value: ctx.format(msg, args, errors),
attrs: null,
};
if (msg.attrs) {
formatted.attrs = [];
- for (const attrName in msg.attrs) {
- const formattedAttr = ctx.format(msg.attrs[attrName], args, errors);
- if (formattedAttr !== null) {
- formatted.attrs.push([
- attrName,
- formattedAttr
- ]);
+ for (const name in msg.attrs) {
+ const value = ctx.format(msg.attrs[name], args, errors);
+ if (value !== null) {
+ formatted.attrs.push({ name, value });
}
}
}
return formatted;
}
/**
--- a/intl/l10n/test/chrome.ini
+++ b/intl/l10n/test/chrome.ini
@@ -1,12 +1,16 @@
+[dom/test_domloc_attr_sanitized.html]
[dom/test_domloc_getAttributes.html]
[dom/test_domloc_setAttributes.html]
[dom/test_domloc_translateElements.html]
[dom/test_domloc_translateFragment.html]
[dom/test_domloc_connectRoot.html]
[dom/test_domloc_disconnectRoot.html]
+[dom/test_domloc_repeated_l10nid.html]
[dom/test_domloc_translateRoots.html]
[dom/test_domloc_mutations.html]
[dom/test_domloc_overlay.html]
[dom/test_domloc_overlay_repeated.html]
+[dom/test_domloc_overlay_missing_all.html]
[dom/test_domloc_overlay_missing_children.html]
+[dom/test_domloc_overlay_sanitized.html]
[dom/test_domloc.xul]
new file mode 100644
--- /dev/null
+++ b/intl/l10n/test/dom/test_domloc_attr_sanitized.html
@@ -0,0 +1,54 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+ <meta charset="utf-8">
+ <title>Test DOMLocalization's attr sanitization functionality</title>
+ <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+ <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+ <script type="application/javascript">
+ "use strict";
+ const { DOMLocalization } =
+ ChromeUtils.import("resource://gre/modules/DOMLocalization.jsm", {});
+ const { MessageContext } =
+ ChromeUtils.import("resource://gre/modules/MessageContext.jsm", {});
+
+ async function* mockGenerateMessages(locales, resourceIds) {
+ const mc = new MessageContext(locales);
+ mc.addMessages(`
+key1 = Value for Key 1
+
+key2 = Value for <a>Key 2<a/>.
+ `);
+ yield mc;
+ }
+
+ SimpleTest.waitForExplicitFinish();
+ addLoadEvent(async () => {
+ const domLoc = new DOMLocalization(
+ window,
+ [],
+ mockGenerateMessages
+ );
+
+ await domLoc.translateFragment(document.body);
+
+ const elem1 = document.querySelector("#elem1");
+ const elem2 = document.querySelector("#elem2");
+
+ ok(elem1.textContent.includes("Value for"));
+ // This is a limitation of us using Node.localize API
+ // Documenting it here to make sure we notice when we fix it
+ is(elem1.getAttribute("title"), "Old Translation");
+
+ ok(elem2.textContent.includes("Value for"));
+ ok(!elem2.hasAttribute("title"));
+
+ SimpleTest.finish();
+ });
+ </script>
+</head>
+<body>
+ <p id="elem1" title="Old Translation" data-l10n-id="key1"></p>
+ <p id="elem2" title="Old Translation" data-l10n-id="key2"></p>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/intl/l10n/test/dom/test_domloc_overlay_missing_all.html
@@ -0,0 +1,46 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+ <meta charset="utf-8">
+ <title>Test DOMLocalization's DOMOverlay functionality</title>
+ <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+ <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+ <script type="application/javascript">
+ "use strict";
+ const { DOMLocalization } =
+ ChromeUtils.import("resource://gre/modules/DOMLocalization.jsm", {});
+ const { MessageContext } =
+ ChromeUtils.import("resource://gre/modules/MessageContext.jsm", {});
+
+ async function* mockGenerateMessages(locales, resourceIds) {
+ const mc = new MessageContext(locales);
+ // No translations!
+ yield mc;
+ }
+
+ SimpleTest.waitForExplicitFinish();
+ addLoadEvent(async () => {
+
+ const domLoc = new DOMLocalization(
+ window,
+ [],
+ mockGenerateMessages
+ );
+
+ await domLoc.translateFragment(document.body);
+
+ // we just care that it doesn't throw here.
+ ok(1);
+
+ SimpleTest.finish();
+ });
+ </script>
+</head>
+<body>
+ <p data-l10n-id="title">
+ <a href="http://www.mozilla.org"></a>
+ <a href="http://www.firefox.com"></a>
+ <a href="http://www.w3.org"></a>
+ </p>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/intl/l10n/test/dom/test_domloc_overlay_sanitized.html
@@ -0,0 +1,55 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+ <meta charset="utf-8">
+ <title>Test DOMLocalization's DOMOverlay functionality</title>
+ <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+ <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+ <script type="application/javascript">
+ "use strict";
+ const { DOMLocalization } =
+ ChromeUtils.import("resource://gre/modules/DOMLocalization.jsm", {});
+ const { MessageContext } =
+ ChromeUtils.import("resource://gre/modules/MessageContext.jsm", {});
+
+ async function* mockGenerateMessages(locales, resourceIds) {
+ const mc = new MessageContext(locales);
+ mc.addMessages(`
+key1 =
+ .href = https://www.hacked.com
+
+key2 =
+ .href = https://pl.wikipedia.org
+`);
+ yield mc;
+ }
+
+ async function test() {
+ const domLoc = new DOMLocalization(
+ window,
+ [],
+ mockGenerateMessages
+ );
+
+ await domLoc.translateFragment(document.body);
+
+ const key1Elem = document.querySelector("[data-l10n-id=key1]");
+ const key2Elem = document.querySelector("[data-l10n-id=key2]");
+
+
+ is(key1Elem.hasAttribute("href"), false, "href translation should not be allowed");
+ is(key2Elem.getAttribute("href"), "https://pl.wikipedia.org",
+ "href translation should be allowed");
+
+ SimpleTest.finish();
+ }
+
+ SimpleTest.waitForExplicitFinish();
+ addLoadEvent(test);
+ </script>
+</head>
+<body>
+ <a data-l10n-id="key1"></a>
+ <a data-l10n-id="key2" data-l10n-attrs="href"></a>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/intl/l10n/test/dom/test_domloc_repeated_l10nid.html
@@ -0,0 +1,63 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+ <meta charset="utf-8">
+ <title>Test DOMLocalization's matching l10nIds functionality</title>
+ <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+ <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+ <script type="application/javascript">
+ "use strict";
+ const { DOMLocalization } =
+ ChromeUtils.import("resource://gre/modules/DOMLocalization.jsm", {});
+ const { MessageContext } =
+ ChromeUtils.import("resource://gre/modules/MessageContext.jsm", {});
+
+ async function* mockGenerateMessages(locales, resourceIds) {
+ const mc = new MessageContext(locales);
+ mc.addMessages(`
+key1 = Translation For Key 1
+
+key2 = Visit <a>this link<a/>.
+ `);
+ yield mc;
+ }
+
+ SimpleTest.waitForExplicitFinish();
+ addLoadEvent(async () => {
+ const domLoc = new DOMLocalization(
+ window,
+ [],
+ mockGenerateMessages
+ );
+
+ await domLoc.translateFragment(document.body);
+
+ ok(document.querySelector("#elem1").textContent.includes("Key 1"));
+ ok(document.querySelector("#elem2").textContent.includes("Key 1"));
+
+ const elem3 = document.querySelector("#elem3");
+ const elem4 = document.querySelector("#elem4");
+
+ ok(elem3.textContent.includes("Visit"));
+ is(elem3.querySelector("a").getAttribute("href"), "http://www.mozilla.org");
+
+ ok(elem4.textContent.includes("Visit"));
+ is(elem4.querySelector("a").getAttribute("href"), "http://www.firefox.com");
+
+ SimpleTest.finish();
+ });
+ </script>
+</head>
+<body>
+ <h1 id="elem1" data-l10n-id="key1"></h1>
+ <h2 id="elem2" data-l10n-id="key1"></h2>
+
+ <p id="elem3" data-l10n-id="key2">
+ <a href="http://www.mozilla.org"></a>
+ </p>
+
+ <p id="elem4" data-l10n-id="key2">
+ <a href="http://www.firefox.com"></a>
+ </p>
+</body>
+</html>