Backout bug 1359780 because it causes bug 1360230, r=backout
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Thu, 27 Apr 2017 20:50:21 +0300
changeset 355159 6518bd1e2c8cc39fc0bbe6b174e7758059c0bd38
parent 355158 0334bfc886c8b6bef2df8c9a130cde364bd5d816
child 355160 2cedd1aab52c76a8f39bc9bf57a3798c0129353c
push id89651
push useropettay@mozilla.com
push dateThu, 27 Apr 2017 17:52:42 +0000
treeherdermozilla-inbound@6518bd1e2c8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1359780, 1360230
milestone55.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
Backout bug 1359780 because it causes bug 1360230, r=backout
dom/base/nsDOMTokenList.cpp
testing/web-platform/mozilla/tests/dom/classList.html
testing/web-platform/tests/dom/nodes/Element-classlist.html
testing/web-platform/tests/dom/nodes/MutationObserver-attributes.html
--- a/dom/base/nsDOMTokenList.cpp
+++ b/dom/base/nsDOMTokenList.cpp
@@ -295,39 +295,26 @@ nsDOMTokenList::Toggle(const nsAString& 
   const nsAttrValue* attr = GetParsedAttr();
   const bool forceOn = aForce.WasPassed() && aForce.Value();
   const bool forceOff = aForce.WasPassed() && !aForce.Value();
 
   bool isPresent = attr && attr->Contains(aToken);
   AutoTArray<nsString, 1> tokens;
   (*tokens.AppendElement()).Rebind(aToken.Data(), aToken.Length());
 
-  if (isPresent && !forceOn) {
-    RemoveInternal(attr, tokens);
-    return false;
-  }
-
-  if (!isPresent && !forceOff) {
-    AddInternal(attr, tokens);
-    return true;
-  }
-
-  if (attr) {
-    // Remove duplicates and whitespace from attr
-    RemoveDuplicates(attr);
-
-    nsAutoString resultStr;
-    for (uint32_t i = 0; i < attr->GetAtomCount(); i++) {
-      if (!resultStr.IsEmpty()) {
-        resultStr.AppendLiteral(" ");
-      }
-      resultStr.Append(nsDependentAtomString(attr->AtomAt(i)));
+  if (isPresent) {
+    if (!forceOn) {
+      RemoveInternal(attr, tokens);
+      isPresent = false;
     }
-
-    mElement->SetAttr(kNameSpaceID_None, mAttrAtom, resultStr, true);
+  } else {
+    if (!forceOff) {
+      AddInternal(attr, tokens);
+      isPresent = true;
+    }
   }
 
   return isPresent;
 }
 
 void
 nsDOMTokenList::Replace(const nsAString& aToken,
                         const nsAString& aNewToken,
@@ -383,17 +370,19 @@ nsDOMTokenList::ReplaceInternal(const ns
       continue;
     }
     if (!resultStr.IsEmpty()) {
       resultStr.AppendLiteral(" ");
     }
     resultStr.Append(nsDependentAtomString(aAttr->AtomAt(i)));
   }
 
-  mElement->SetAttr(kNameSpaceID_None, mAttrAtom, resultStr, true);
+  if (sawIt) {
+    mElement->SetAttr(kNameSpaceID_None, mAttrAtom, resultStr, true);
+  }
 }
 
 bool
 nsDOMTokenList::Supports(const nsAString& aToken,
                          ErrorResult& aError)
 {
   if (!mSupportedTokens) {
     aError.ThrowTypeError<MSG_TOKENLIST_NO_SUPPORTED_TOKENS>(
--- a/testing/web-platform/mozilla/tests/dom/classList.html
+++ b/testing/web-platform/mozilla/tests/dom/classList.html
@@ -249,20 +249,22 @@ function testClassList(e, desc) {
   checkContains("null undefined", [null, undefined], true);
   checkContains("\t\n\f\r a\t\n\f\r b\t\n\f\r ", ["a", "b"], true);
 
   // add() method
 
   function checkAdd(before, argument, after, expectedException) {
     checkModification(e, "add", argument, undefined, before, after,
                       expectedException, desc);
-    if (!Array.isArray(argument)) {
-      checkModification(e, "toggle", [argument, true], true, before, after,
-                        expectedException, desc);
-    }
+    // Also check force toggle
+    // XXX https://github.com/whatwg/dom/issues/443
+    //if (!Array.isArray(argument)) {
+    //  checkModification(e, "toggle", [argument, true], true, before, after,
+    //                    expectedException);
+    //}
   }
 
   checkAdd(null, "", null, "SyntaxError");
   checkAdd(null, ["a", ""], null, "SyntaxError");
   checkAdd(null, " ", null, "InvalidCharacterError");
   checkAdd(null, "\ta", null, "InvalidCharacterError");
   checkAdd(null, "a\t", null, "InvalidCharacterError");
   checkAdd(null, "\na", null, "InvalidCharacterError");
@@ -302,20 +304,22 @@ function testClassList(e, desc) {
   checkAdd(null, null, "null");
   checkAdd(null, undefined, "undefined");
 
   // remove() method
 
   function checkRemove(before, argument, after, expectedException) {
     checkModification(e, "remove", argument, undefined, before, after,
                       expectedException, desc);
-    if (!Array.isArray(argument)) {
-      checkModification(e, "toggle", [argument, false], false, before, after,
-                        expectedException, desc);
-    }
+    // Also check force toggle
+    // XXX https://github.com/whatwg/dom/issues/443
+    //if (!Array.isArray(argument)) {
+    //  checkModification(e, "toggle", [argument, false], false, before, after,
+    //                    expectedException);
+    //}
   }
 
   checkRemove(null, "", null, "SyntaxError");
   checkRemove(null, " ", null, "InvalidCharacterError");
   checkRemove("\ta", "\ta", "\ta", "InvalidCharacterError");
   checkRemove("a\t", "a\t", "a\t", "InvalidCharacterError");
   checkRemove("\na", "\na", "\na", "InvalidCharacterError");
   checkRemove("a\n", "a\n", "a\n", "InvalidCharacterError");
@@ -398,16 +402,34 @@ function testClassList(e, desc) {
   checkToggle("\t\n\f\r a\t\n\f\r b\t\n\f\r ", "c", true, "a b c");
 
   checkToggle("null", null, false, "");
   checkToggle("", null, true, "null");
   checkToggle("undefined", undefined, false, "");
   checkToggle("", undefined, true, "undefined");
 
 
+  // tests for the force argument handling
+  // XXX Remove these if https://github.com/whatwg/dom/issues/443 is fixed
+
+  function checkForceToggle(before, argument, force, expectedRes, after, expectedException) {
+    checkModification(e, "toggle", [argument, force], expectedRes, before,
+                      after, expectedException, desc);
+  }
+
+  checkForceToggle("", "a", true, true, "a");
+  checkForceToggle("a", "a", true, true, "a");
+  checkForceToggle("a", "b", true, true, "a b");
+  checkForceToggle("a b", "b", true, true, "a b");
+  checkForceToggle("", "a", false, false, "");
+  checkForceToggle("a", "a", false, false, "");
+  checkForceToggle("a", "b", false, false, "a");
+  checkForceToggle("a b", "b", false, false, "a");
+
+
   // replace() method
   function checkReplace(before, token, newToken, after, expectedException) {
     checkModification(e, "replace", [token, newToken], undefined, before,
                       after, expectedException, desc);
   }
 
   checkReplace(null, "", "a", null, "SyntaxError");
   checkReplace(null, "", " ", null, "SyntaxError");
@@ -437,25 +459,29 @@ function testClassList(e, desc) {
   checkReplace(null, "b", " a", null, "InvalidCharacterError");
   checkReplace(null, "b", "a ", null, "InvalidCharacterError");
 
   checkReplace("a", "a", "a", "a");
   checkReplace("a", "a", "b", "b");
   checkReplace("a", "A", "b", "a");
   checkReplace("a b", "b", "A", "a A");
   checkReplace("a b c", "d", "e", "a b c");
+  // https://github.com/whatwg/dom/issues/443
   checkReplace("a a a  b", "a", "a", "a b");
-  checkReplace("a a a  b", "c", "d", "a b");
+  checkReplace("a a a  b", "c", "d", "a a a  b");
   checkReplace(null, "a", "b", null);
   checkReplace("", "a", "b", "");
-  checkReplace(" ", "a", "b", "");
+  checkReplace(" ", "a", "b", " ");
   checkReplace(" a  \f", "a", "b", "b");
   checkReplace("a b c", "b", "d", "a d c");
+  // https://github.com/whatwg/dom/issues/442
+  // Implementations agree on the first one here, so I test it, but disagree on
+  // the second, so no test until the spec decides what to say.
   checkReplace("a b c", "c", "a", "a b");
-  checkReplace("c b a", "c", "a", "a b");
+  //checkReplace("c b a", "c", "a", ???);
   checkReplace("a b a", "a", "c", "c b");
   checkReplace("a b a", "b", "c", "a c");
   checkReplace("   a  a b", "a", "c", "c b");
   checkReplace("   a  a b", "b", "c", "a c");
   checkReplace("\t\n\f\r a\t\n\f\r b\t\n\f\r ", "a", "c", "c b");
   checkReplace("\t\n\f\r a\t\n\f\r b\t\n\f\r ", "b", "c", "a c");
 
   checkReplace("a null", null, "b", "a b");
--- a/testing/web-platform/tests/dom/nodes/Element-classlist.html
+++ b/testing/web-platform/tests/dom/nodes/Element-classlist.html
@@ -197,20 +197,22 @@ function testClassList(e, desc) {
   checkContains("null undefined", [null, undefined], true);
   checkContains("\t\n\f\r a\t\n\f\r b\t\n\f\r ", ["a", "b"], true);
 
   // add() method
 
   function checkAdd(before, argument, after, expectedException) {
     checkModification(e, "add", argument, undefined, before, after,
                       expectedException, desc);
-    if (!Array.isArray(argument)) {
-      checkModification(e, "toggle", [argument, true], true, before, after,
-                        expectedException, desc);
-    }
+    // Also check force toggle
+    // XXX https://github.com/whatwg/dom/issues/443
+    //if (!Array.isArray(argument)) {
+    //  checkModification(e, "toggle", [argument, true], true, before, after,
+    //                    expectedException);
+    //}
   }
 
   checkAdd(null, "", null, "SyntaxError");
   checkAdd(null, ["a", ""], null, "SyntaxError");
   checkAdd(null, " ", null, "InvalidCharacterError");
   checkAdd(null, "\ta", null, "InvalidCharacterError");
   checkAdd(null, "a\t", null, "InvalidCharacterError");
   checkAdd(null, "\na", null, "InvalidCharacterError");
@@ -250,20 +252,22 @@ function testClassList(e, desc) {
   checkAdd(null, null, "null");
   checkAdd(null, undefined, "undefined");
 
   // remove() method
 
   function checkRemove(before, argument, after, expectedException) {
     checkModification(e, "remove", argument, undefined, before, after,
                       expectedException, desc);
-    if (!Array.isArray(argument)) {
-      checkModification(e, "toggle", [argument, false], false, before, after,
-                        expectedException, desc);
-    }
+    // Also check force toggle
+    // XXX https://github.com/whatwg/dom/issues/443
+    //if (!Array.isArray(argument)) {
+    //  checkModification(e, "toggle", [argument, false], false, before, after,
+    //                    expectedException);
+    //}
   }
 
   checkRemove(null, "", null, "SyntaxError");
   checkRemove(null, " ", null, "InvalidCharacterError");
   checkRemove("\ta", "\ta", "\ta", "InvalidCharacterError");
   checkRemove("a\t", "a\t", "a\t", "InvalidCharacterError");
   checkRemove("\na", "\na", "\na", "InvalidCharacterError");
   checkRemove("a\n", "a\n", "a\n", "InvalidCharacterError");
@@ -346,16 +350,34 @@ function testClassList(e, desc) {
   checkToggle("\t\n\f\r a\t\n\f\r b\t\n\f\r ", "c", true, "a b c");
 
   checkToggle("null", null, false, "");
   checkToggle("", null, true, "null");
   checkToggle("undefined", undefined, false, "");
   checkToggle("", undefined, true, "undefined");
 
 
+  // tests for the force argument handling
+  // XXX Remove these if https://github.com/whatwg/dom/issues/443 is fixed
+
+  function checkForceToggle(before, argument, force, expectedRes, after, expectedException) {
+    checkModification(e, "toggle", [argument, force], expectedRes, before,
+                      after, expectedException, desc);
+  }
+
+  checkForceToggle("", "a", true, true, "a");
+  checkForceToggle("a", "a", true, true, "a");
+  checkForceToggle("a", "b", true, true, "a b");
+  checkForceToggle("a b", "b", true, true, "a b");
+  checkForceToggle("", "a", false, false, "");
+  checkForceToggle("a", "a", false, false, "");
+  checkForceToggle("a", "b", false, false, "a");
+  checkForceToggle("a b", "b", false, false, "a");
+
+
   // replace() method
   function checkReplace(before, token, newToken, after, expectedException) {
     checkModification(e, "replace", [token, newToken], undefined, before,
                       after, expectedException, desc);
   }
 
   checkReplace(null, "", "a", null, "SyntaxError");
   checkReplace(null, "", " ", null, "SyntaxError");
@@ -385,25 +407,29 @@ function testClassList(e, desc) {
   checkReplace(null, "b", " a", null, "InvalidCharacterError");
   checkReplace(null, "b", "a ", null, "InvalidCharacterError");
 
   checkReplace("a", "a", "a", "a");
   checkReplace("a", "a", "b", "b");
   checkReplace("a", "A", "b", "a");
   checkReplace("a b", "b", "A", "a A");
   checkReplace("a b c", "d", "e", "a b c");
+  // https://github.com/whatwg/dom/issues/443
   checkReplace("a a a  b", "a", "a", "a b");
-  checkReplace("a a a  b", "c", "d", "a b");
+  checkReplace("a a a  b", "c", "d", "a a a  b");
   checkReplace(null, "a", "b", null);
   checkReplace("", "a", "b", "");
-  checkReplace(" ", "a", "b", "");
+  checkReplace(" ", "a", "b", " ");
   checkReplace(" a  \f", "a", "b", "b");
   checkReplace("a b c", "b", "d", "a d c");
+  // https://github.com/whatwg/dom/issues/442
+  // Implementations agree on the first one here, so I test it, but disagree on
+  // the second, so no test until the spec decides what to say.
   checkReplace("a b c", "c", "a", "a b");
-  checkReplace("c b a", "c", "a", "a b");
+  //checkReplace("c b a", "c", "a", ???);
   checkReplace("a b a", "a", "c", "c b");
   checkReplace("a b a", "b", "c", "a c");
   checkReplace("   a  a b", "a", "c", "c b");
   checkReplace("   a  a b", "b", "c", "a c");
   checkReplace("\t\n\f\r a\t\n\f\r b\t\n\f\r ", "a", "c", "c b");
   checkReplace("\t\n\f\r a\t\n\f\r b\t\n\f\r ", "b", "c", "a c");
 
   checkReplace("a null", null, "b", "a b");
--- a/testing/web-platform/tests/dom/nodes/MutationObserver-attributes.html
+++ b/testing/web-platform/tests/dom/nodes/MutationObserver-attributes.html
@@ -216,28 +216,26 @@ runMutationTest(n42,
                   {"attributes":true, "attributeOldValue": true},
                   [{type: "attributes", oldValue: "c01 c02", attributeName: "class"}],
                   function() { n42.classList.toggle("c01", false);},
                   "attributes Element.classList.toggle: forced token removal mutation");
 
   var n43 = document.getElementById('n43');
 runMutationTest(n43,
                   {"attributes":true, "attributeOldValue": true},
-                  [{type: "attributes", oldValue: "c01 c02", attributeName: "class"},
-                   {type: "attributes", oldValue: "n43", attributeName: "id"}],
+                  [{type: "attributes", oldValue: "n43", attributeName: "id"}],
                   function() { n43.classList.toggle("c03", false); n43.id = "n430"; },
-                  "attributes Element.classList.toggle: forced missing token removal no-op");
+                  "attributes Element.classList.toggle: forced missing token removal no mutation");
 
   var n44 = document.getElementById('n44');
 runMutationTest(n44,
                   {"attributes":true, "attributeOldValue": true},
-                  [{type: "attributes", oldValue: "c01 c02", attributeName: "class"},
-                   {type: "attributes", oldValue: "n44", attributeName: "id"}],
+                  [{type: "attributes", oldValue: "n44", attributeName: "id"}],
                   function() { n44.classList.toggle("c01", true); n44.id = "n440"; },
-                  "attributes Element.classList.toggle: forced existing token addition no-op");
+                  "attributes Element.classList.toggle: forced existing token addition no mutation");
 
   var n45 = document.getElementById('n45');
 runMutationTest(n45,
                   {"attributes":true, "attributeOldValue": true},
                   [{type: "attributes", oldValue: "c01 c02", attributeName: "class"}],
                   function() { n45.classList.toggle("c03", true);},
                   "attributes Element.classList.toggle: forced token addition mutation");