Backed out changeset a8acb6aa4a52 (bug 1534480)for chrome mochitest failures on dom_overlays/test_same_attribute.html CLOSED TREE
authorarthur.iakab <aiakab@mozilla.com>
Tue, 16 Apr 2019 20:54:11 +0300
changeset 469720 140dfb5fd701
parent 469719 50efd1672ffb
child 469721 12a60898fdc1
child 469722 9066ae618a1e
push id35879
push usernerli@mozilla.com
push dateTue, 16 Apr 2019 22:01:48 +0000
treeherdermozilla-central@12a60898fdc1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1534480
milestone68.0a1
backs outa8acb6aa4a52
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
Backed out changeset a8acb6aa4a52 (bug 1534480)for chrome mochitest failures on dom_overlays/test_same_attribute.html CLOSED TREE
dom/base/nsINode.cpp
intl/l10n/DOMLocalization.jsm
intl/l10n/DocumentL10n.cpp
intl/l10n/test/chrome.ini
intl/l10n/test/dom_overlays/test_same_attribute.html
intl/l10n/test/dom_overlays/test_same_id.html
intl/l10n/test/dom_overlays/test_same_id_args.html
--- a/dom/base/nsINode.cpp
+++ b/dom/base/nsINode.cpp
@@ -2788,26 +2788,22 @@ class LocalizationHandler : public Promi
           return;
         }
       }
 
       Nullable<Sequence<AttributeNameValue>>& attributes =
           l10nData[i].mAttributes;
       if (!attributes.IsNull()) {
         for (size_t j = 0; j < attributes.Value().Length(); ++j) {
-          nsString& name = attributes.Value()[j].mName;
-          nsString& value = attributes.Value()[j].mValue;
-          RefPtr<nsAtom> nameAtom = NS_Atomize(name);
-          if (!elem->AttrValueIs(kNameSpaceID_None, nameAtom, value,
-                                 eCaseMatters)) {
-            rv = elem->SetAttr(kNameSpaceID_None, nameAtom, value, true);
-            if (rv.Failed()) {
-              mReturnValuePromise->MaybeRejectWithUndefined();
-              return;
-            }
+          // Use SetAttribute here to validate the attribute name!
+          elem->SetAttribute(attributes.Value()[j].mName,
+                             attributes.Value()[j].mValue, rv);
+          if (rv.Failed()) {
+            mReturnValuePromise->MaybeRejectWithUndefined();
+            return;
           }
         }
       }
 
       if (content.IsVoid() && attributes.IsNull()) {
         JS::Rooted<JS::Value> wrappedElem(aCx);
         if (!ToJSValue(aCx, elem, &wrappedElem)) {
           mReturnValuePromise->MaybeRejectWithUndefined();
--- a/intl/l10n/DOMLocalization.jsm
+++ b/intl/l10n/DOMLocalization.jsm
@@ -146,64 +146,49 @@ function overlayChildNodes(fromFragment,
     // If all else fails, replace the element with its text content.
     fromFragment.replaceChild(textNode(childNode), childNode);
   }
 
   toElement.textContent = "";
   toElement.appendChild(fromFragment);
 }
 
-function hasAttribute(attributes, name) {
-  if (!attributes) {
-    return false;
-  }
-  for (let attr of attributes) {
-    if (attr.name === name) {
-      return true;
-    }
-  }
-  return false;
-}
-
 /**
  * Transplant localizable attributes of an element to another element.
  *
  * Any localizable attributes already set on the target element will be
  * cleared.
  *
  * @param   {Element|Object} fromElement - The source of child nodes to overlay.
  * @param   {Element} toElement - The target of the overlay.
  * @private
  */
 function overlayAttributes(fromElement, toElement) {
   const explicitlyAllowed = toElement.hasAttribute("data-l10n-attrs")
     ? toElement.getAttribute("data-l10n-attrs")
       .split(",").map(i => i.trim())
     : null;
 
-  // Remove existing localizable attributes if they
-  // will not be used in the new translation.
+  // Remove existing localizable attributes.
   for (const attr of Array.from(toElement.attributes)) {
-    if (isAttrNameLocalizable(attr.name, toElement, explicitlyAllowed)
-      && !hasAttribute(fromElement.attributes, attr.name)) {
+    if (isAttrNameLocalizable(attr.name, toElement, explicitlyAllowed)) {
       toElement.removeAttribute(attr.name);
     }
   }
 
   // fromElement might be a {value, attributes} object as returned by
   // Localization.messageFromBundle. In which case attributes may be null to
   // save GC cycles.
   if (!fromElement.attributes) {
     return;
   }
 
   // Set localizable attributes.
   for (const attr of Array.from(fromElement.attributes)) {
-    if (isAttrNameLocalizable(attr.name, toElement, explicitlyAllowed)
-      && toElement.getAttribute(attr.name) !== attr.value) {
+    if (isAttrNameLocalizable(attr.name, toElement, explicitlyAllowed)) {
       toElement.setAttribute(attr.name, attr.value);
     }
   }
 }
 
 /**
  * Sanitize a child element created by the translation.
  *
@@ -485,25 +470,20 @@ class DOMLocalization extends Localizati
    * </p>
    * ```
    *
    * @param {Element}                element - Element to set attributes on
    * @param {string}                 id      - l10n-id string
    * @param {Object<string, string>} args    - KVP list of l10n arguments
    * @returns {Element}
    */
-  setAttributes(element, id, args = null) {
-    if (element.getAttribute(L10NID_ATTR_NAME) !== id) {
-      element.setAttribute(L10NID_ATTR_NAME, id);
-    }
+  setAttributes(element, id, args) {
+    element.setAttribute(L10NID_ATTR_NAME, id);
     if (args) {
-      let argsString = JSON.stringify(args);
-      if (argsString !== element.getAttribute(L10NARGS_ATTR_NAME)) {
-        element.setAttribute(L10NARGS_ATTR_NAME, argsString);
-      }
+      element.setAttribute(L10NARGS_ATTR_NAME, JSON.stringify(args));
     } else {
       element.removeAttribute(L10NARGS_ATTR_NAME);
     }
     return element;
   }
 
   /**
    * Get the `data-l10n-*` attributes from DOM elements.
--- a/intl/l10n/DocumentL10n.cpp
+++ b/intl/l10n/DocumentL10n.cpp
@@ -258,34 +258,30 @@ already_AddRefed<Promise> DocumentL10n::
   }
   return MaybeWrapPromise(promise);
 }
 
 void DocumentL10n::SetAttributes(JSContext* aCx, Element& aElement,
                                  const nsAString& aId,
                                  const Optional<JS::Handle<JSObject*>>& aArgs,
                                  ErrorResult& aRv) {
-  if (!aElement.AttrValueIs(kNameSpaceID_None, nsGkAtoms::datal10nid, aId,
-                            eCaseMatters)) {
-    aElement.SetAttr(kNameSpaceID_None, nsGkAtoms::datal10nid, aId, true);
+  aElement.SetAttribute(NS_LITERAL_STRING("data-l10n-id"), aId, aRv);
+  if (aRv.Failed()) {
+    return;
   }
-
   if (aArgs.WasPassed()) {
     nsAutoString data;
     JS::Rooted<JS::Value> val(aCx, JS::ObjectValue(*aArgs.Value()));
     if (!nsContentUtils::StringifyJSON(aCx, &val, data)) {
       aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);
       return;
     }
-    if (!aElement.AttrValueIs(kNameSpaceID_None, nsGkAtoms::datal10nargs, data,
-                              eCaseMatters)) {
-      aElement.SetAttr(kNameSpaceID_None, nsGkAtoms::datal10nargs, data, true);
-    }
+    aElement.SetAttribute(NS_LITERAL_STRING("data-l10n-args"), data, aRv);
   } else {
-    aElement.UnsetAttr(kNameSpaceID_None, nsGkAtoms::datal10nargs, true);
+    aElement.RemoveAttribute(NS_LITERAL_STRING("data-l10n-args"), aRv);
   }
 }
 
 void DocumentL10n::GetAttributes(JSContext* aCx, Element& aElement,
                                  L10nKey& aResult, ErrorResult& aRv) {
   nsAutoString l10nId;
   nsAutoString l10nArgs;
 
--- a/intl/l10n/test/chrome.ini
+++ b/intl/l10n/test/chrome.ini
@@ -21,12 +21,8 @@
 
 [dom/test_docl10n.xul]
 [dom/test_docl10n_initialize_after_parse.xhtml]
 [dom/test_docl10n_initialize_after_parse.xul]
 [dom/test_docl10n.xhtml]
 [dom/test_docl10n.html]
 [dom/test_docl10n_ready_rejected.html]
 [dom/test_docl10n_removeResourceIds.html]
-
-[dom_overlays/test_same_id.html]
-[dom_overlays/test_same_id_args.html]
-[dom_overlays/test_same_attribute.html]
deleted file mode 100644
--- a/intl/l10n/test/dom_overlays/test_same_attribute.html
+++ /dev/null
@@ -1,61 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <meta charset="utf-8">
-  <title>Test Amount of mutations generated from DOM Overlays</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">
-  <link rel="localization" href="browser/preferences/preferences.ftl"/>
-  <script type="application/javascript">
-  "use strict";
-  SimpleTest.waitForExplicitFinish();
-
-  let config = {
-    attributes: true,
-    attributeOldValue: true,
-    characterData: true,
-    characterDataOldValue: true,
-    childList: true,
-    subtree: true,
-  };
-  let allMutations = [];
-
-  document.addEventListener("DOMContentLoaded", async function() {
-    await document.l10n.ready;
-
-    let optionElem = document.getElementById("use-current-pages");
-
-    // Test for initial localization applied.
-    is(optionElem.getAttribute("label").length > 0, true);
-
-    let observer = new MutationObserver((mutations) => {
-      for (let mutation of mutations) {
-        allMutations.push(mutation);
-      }
-    });
-    observer.observe(optionElem, config);
-
-    document.l10n.setAttributes(optionElem, "use-current-pages", {
-      tabCount: 6,
-    });
-
-    // Due to the async iteractions between nsINode.localize
-    // and DOMLocalization.jsm, we'll need to wait two frames
-    // to verify that no mutations happened.
-    requestAnimationFrame(() => {
-      requestAnimationFrame(() => {
-        // We only expect the `data-l10n-args` attribute
-        // to be updated and cause a single mutation.
-        // Since the result value of the attribute stays
-        // the same, there should be no mutation fired for it.
-        is(allMutations.length, 1);
-        SimpleTest.finish();
-      });
-    });
-  }, { once: true});
-  </script>
-</head>
-<body>
-  <option id="use-current-pages" data-l10n-id="use-current-pages" data-l10n-args='{"tabCount":5}'></option>
-</body>
-</html>
deleted file mode 100644
--- a/intl/l10n/test/dom_overlays/test_same_id.html
+++ /dev/null
@@ -1,57 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <meta charset="utf-8">
-  <title>Test Amount of mutations generated from DOM Overlays</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">
-  <link rel="localization" href="browser/aboutConfig.ftl"/>
-  <script type="application/javascript">
-  "use strict";
-  SimpleTest.waitForExplicitFinish();
-
-  let config = {
-    attributes: true,
-    attributeOldValue: true,
-    characterData: true,
-    characterDataOldValue: true,
-    childList: true,
-    subtree: true,
-  };
-  let allMutations = [];
-
-  document.addEventListener("DOMContentLoaded", async function() {
-    await document.l10n.ready;
-
-    let inputElem = document.getElementById("search-input");
-
-    // Test for initial localization applied.
-    is(inputElem.getAttribute("placeholder").length > 0, true);
-
-    let observer = new MutationObserver((mutations) => {
-      for (let mutation of mutations) {
-        allMutations.push(mutation);
-      }
-    });
-    observer.observe(inputElem, config);
-
-    document.l10n.setAttributes(inputElem, "about-config-search-input");
-
-    // Due to the async iteractions between nsINode.localize
-    // and DOMLocalization.jsm, we'll need to wait two frames
-    // to verify that no mutations happened.
-    requestAnimationFrame(() => {
-      requestAnimationFrame(() => {
-        // Since the l10n-id is the same as the previous one
-        // no mutation should happen in result.
-        is(allMutations.length, 0);
-        SimpleTest.finish();
-      });
-    });
-  }, { once: true});
-  </script>
-</head>
-<body>
-  <input id="search-input" data-l10n-id="about-config-search-input"></input>
-</body>
-</html>
deleted file mode 100644
--- a/intl/l10n/test/dom_overlays/test_same_id_args.html
+++ /dev/null
@@ -1,57 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <meta charset="utf-8">
-  <title>Test Amount of mutations generated from DOM Overlays</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">
-  <link rel="localization" href="browser/aboutConfig.ftl"/>
-  <script type="application/javascript">
-  "use strict";
-  SimpleTest.waitForExplicitFinish();
-
-  let config = {
-    attributes: true,
-    attributeOldValue: true,
-    characterData: true,
-    characterDataOldValue: true,
-    childList: true,
-    subtree: true,
-  };
-  let allMutations = [];
-
-  document.addEventListener("DOMContentLoaded", async function() {
-    await document.l10n.ready;
-
-    let inputElem = document.getElementById("search-input");
-
-    // Test for initial localization applied.
-    is(inputElem.getAttribute("aria-label").length > 0, true);
-
-    let observer = new MutationObserver((mutations) => {
-      for (let mutation of mutations) {
-        allMutations.push(mutation);
-      }
-    });
-    observer.observe(inputElem, config);
-
-    document.l10n.setAttributes(inputElem, "about-config-pref-accessible-value-default", {value: "Test"});
-
-    // Due to the async iteractions between nsINode.localize
-    // and DOMLocalization.jsm, we'll need to wait two frames
-    // to verify that no mutations happened.
-    requestAnimationFrame(() => {
-      requestAnimationFrame(() => {
-        // Since the l10n-id is the same as the previous one
-        // no mutation should happen in result.
-        is(allMutations.length, 0);
-        SimpleTest.finish();
-      });
-    });
-  }, { once: true});
-  </script>
-</head>
-<body>
-  <input id="search-input" data-l10n-id="about-config-pref-accessible-value-default" data-l10n-args='{"value":"Test"}'></input>
-</body>
-</html>