Update DOMTokenList.replace() to match the latest DOM specification
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 28 Apr 2017 20:52:28 +0000 (20:52 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 28 Apr 2017 20:52:28 +0000 (20:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171388

Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

Re-sync web-platform-test after:
- https://github.com/w3c/web-platform-tests/pull/5725

This adds test coverage for the behavior change in this patch.

* web-platform-tests/dom/nodes/Element-classlist.html:

Source/WebCore:

Update DOMTokenList.replace() to match the latest DOM specification after:
- https://github.com/whatwg/dom/issues/442
- https://github.com/whatwg/infra/pull/126

The latest spec text is at:
- https://dom.spec.whatwg.org/#dom-domtokenlist-replace
- https://infra.spec.whatwg.org/#set-replace

The behavior change in this patch causes (a, b, c).replace(a, c) to return
(c, b) instead of (b, c). This new behavior is aligned with Firefox as well.

No new tests, updated existing test.

* html/DOMTokenList.cpp:
(WebCore::DOMTokenList::replace):

Source/WTF:

Add Vector::findMatching() API which takes in a lambda function for convenience.
Add optional startIndex parameter to Vector::removeFirstMatching() / removeAllMatching()
to remove items only after given index.

* wtf/Vector.h:
(WTF::minCapacity>::findMatching):
(WTF::minCapacity>::find):
(WTF::minCapacity>::removeFirstMatching):
(WTF::minCapacity>::removeAllMatching):

Tools:

Add API test coverage for new Vector API.

* TestWebKitAPI/Tests/WTF/Vector.cpp:
(TestWebKitAPI::TEST):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@215943 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/dom/nodes/Element-classlist.html
Source/WTF/ChangeLog
Source/WTF/wtf/Vector.h
Source/WebCore/ChangeLog
Source/WebCore/html/DOMTokenList.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/Vector.cpp

index 3d034d6..fe3ef3d 100644 (file)
@@ -1,5 +1,19 @@
 2017-04-28  Chris Dumez  <cdumez@apple.com>
 
+        Update DOMTokenList.replace() to match the latest DOM specification
+        https://bugs.webkit.org/show_bug.cgi?id=171388
+
+        Reviewed by Alex Christensen.
+
+        Re-sync web-platform-test after:
+        - https://github.com/w3c/web-platform-tests/pull/5725
+
+        This adds test coverage for the behavior change in this patch.
+
+        * web-platform-tests/dom/nodes/Element-classlist.html:
+
+2017-04-28  Chris Dumez  <cdumez@apple.com>
+
         URLSearchParams should be reflective
         https://bugs.webkit.org/show_bug.cgi?id=171345
 
index e986e81..fd82b00 100644 (file)
@@ -3,8 +3,8 @@
   <head class="test test">
     <title class=" ">Element.classList in case-sensitive documents</title>
     <link rel="help" href="https://dom.spec.whatwg.org/#concept-class">
-    <script type="text/javascript" src="../../../../../resources/testharness.js"></script>
-    <script type="text/javascript" src="../../../../../resources/testharnessreport.js"></script>
+    <script type="text/javascript" src="/resources/testharness.js"></script>
+    <script type="text/javascript" src="/resources/testharnessreport.js"></script>
     <style type="text/css">
 .foo { font-style: italic; }
     </style>
@@ -109,6 +109,8 @@ test(function () {
   assert_false( foo.classList.contains('token1') );
   assert_true( foo.classList.contains('token2') );
   assert_true( foo.classList.contains('token3') );
+  assert_equals( foo.classList.item(0), 'token3' );
+  assert_equals( foo.classList.item(1), 'token2' );
 }, '.replace with an already existing token')
 elem.className = 'foo';
 test(function () {
index f77d41e..3bd72be 100644 (file)
@@ -1,3 +1,20 @@
+2017-04-28  Chris Dumez  <cdumez@apple.com>
+
+        Update DOMTokenList.replace() to match the latest DOM specification
+        https://bugs.webkit.org/show_bug.cgi?id=171388
+
+        Reviewed by Alex Christensen.
+
+        Add Vector::findMatching() API which takes in a lambda function for convenience.
+        Add optional startIndex parameter to Vector::removeFirstMatching() / removeAllMatching()
+        to remove items only after given index.
+
+        * wtf/Vector.h:
+        (WTF::minCapacity>::findMatching):
+        (WTF::minCapacity>::find):
+        (WTF::minCapacity>::removeFirstMatching):
+        (WTF::minCapacity>::removeAllMatching):
+
 2017-04-27  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Move UUID from WebCore/platform to WTF
index 9333121..f105d41 100644 (file)
@@ -704,6 +704,7 @@ public:
     
     template<typename U> bool contains(const U&) const;
     template<typename U> size_t find(const U&) const;
+    template<typename MatchFunction> size_t findMatching(const MatchFunction&) const;
     template<typename U> size_t reverseFind(const U&) const;
     
     template<typename U> bool appendIfNotContains(const U&);
@@ -739,9 +740,9 @@ public:
     void remove(size_t position);
     void remove(size_t position, size_t length);
     template<typename U> bool removeFirst(const U&);
-    template<typename MatchFunction> bool removeFirstMatching(const MatchFunction&);
+    template<typename MatchFunction> bool removeFirstMatching(const MatchFunction&, size_t startIndex = 0);
     template<typename U> unsigned removeAll(const U&);
-    template<typename MatchFunction> unsigned removeAllMatching(const MatchFunction&);
+    template<typename MatchFunction> unsigned removeAllMatching(const MatchFunction&, size_t startIndex = 0);
 
     void removeLast() 
     {
@@ -904,11 +905,11 @@ bool Vector<T, inlineCapacity, OverflowHandler, minCapacity>::contains(const U&
 }
 
 template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
-template<typename U>
-size_t Vector<T, inlineCapacity, OverflowHandler, minCapacity>::find(const U& value) const
+template<typename MatchFunction>
+size_t Vector<T, inlineCapacity, OverflowHandler, minCapacity>::findMatching(const MatchFunction& matches) const
 {
     for (size_t i = 0; i < size(); ++i) {
-        if (at(i) == value)
+        if (matches(at(i)))
             return i;
     }
     return notFound;
@@ -916,6 +917,15 @@ size_t Vector<T, inlineCapacity, OverflowHandler, minCapacity>::find(const U& va
 
 template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
 template<typename U>
+size_t Vector<T, inlineCapacity, OverflowHandler, minCapacity>::find(const U& value) const
+{
+    return findMatching([&](auto& item) {
+        return item == value;
+    });
+}
+
+template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
+template<typename U>
 size_t Vector<T, inlineCapacity, OverflowHandler, minCapacity>::reverseFind(const U& value) const
 {
     for (size_t i = 1; i <= size(); ++i) {
@@ -1396,9 +1406,9 @@ inline bool Vector<T, inlineCapacity, OverflowHandler, minCapacity>::removeFirst
 
 template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
 template<typename MatchFunction>
-inline bool Vector<T, inlineCapacity, OverflowHandler, minCapacity>::removeFirstMatching(const MatchFunction& matches)
+inline bool Vector<T, inlineCapacity, OverflowHandler, minCapacity>::removeFirstMatching(const MatchFunction& matches, size_t startIndex)
 {
-    for (size_t i = 0; i < size(); ++i) {
+    for (size_t i = startIndex; i < size(); ++i) {
         if (matches(at(i))) {
             remove(i);
             return true;
@@ -1418,12 +1428,12 @@ inline unsigned Vector<T, inlineCapacity, OverflowHandler, minCapacity>::removeA
 
 template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
 template<typename MatchFunction>
-inline unsigned Vector<T, inlineCapacity, OverflowHandler, minCapacity>::removeAllMatching(const MatchFunction& matches)
+inline unsigned Vector<T, inlineCapacity, OverflowHandler, minCapacity>::removeAllMatching(const MatchFunction& matches, size_t startIndex)
 {
     iterator holeBegin = end();
     iterator holeEnd = end();
     unsigned matchCount = 0;
-    for (auto it = begin(), itEnd = end(); it != itEnd; ++it) {
+    for (auto it = begin() + startIndex, itEnd = end(); it < itEnd; ++it) {
         if (matches(*it)) {
             if (holeBegin == end())
                 holeBegin = it;
index 10cf06f..bf84895 100644 (file)
@@ -1,3 +1,26 @@
+2017-04-28  Chris Dumez  <cdumez@apple.com>
+
+        Update DOMTokenList.replace() to match the latest DOM specification
+        https://bugs.webkit.org/show_bug.cgi?id=171388
+
+        Reviewed by Alex Christensen.
+
+        Update DOMTokenList.replace() to match the latest DOM specification after:
+        - https://github.com/whatwg/dom/issues/442
+        - https://github.com/whatwg/infra/pull/126
+
+        The latest spec text is at:
+        - https://dom.spec.whatwg.org/#dom-domtokenlist-replace
+        - https://infra.spec.whatwg.org/#set-replace
+
+        The behavior change in this patch causes (a, b, c).replace(a, c) to return
+        (c, b) instead of (b, c). This new behavior is aligned with Firefox as well.
+
+        No new tests, updated existing test.
+
+        * html/DOMTokenList.cpp:
+        (WebCore::DOMTokenList::replace):
+
 2017-04-28  Brady Eidson  <beidson@apple.com>
 
         Start of support for multiple WebsiteDataStore/SessionIDs per process
index 0395a2e..3a80cd7 100644 (file)
@@ -158,23 +158,29 @@ ExceptionOr<bool> DOMTokenList::toggle(const AtomicString& token, std::optional<
     return true;
 }
 
-ExceptionOr<void> DOMTokenList::replace(const AtomicString& token, const AtomicString& newToken)
+// https://dom.spec.whatwg.org/#dom-domtokenlist-replace
+ExceptionOr<void> DOMTokenList::replace(const AtomicString& item, const AtomicString& replacement)
 {
-    if (token.isEmpty() || newToken.isEmpty())
+    if (item.isEmpty() || replacement.isEmpty())
         return Exception { SYNTAX_ERR };
 
-    if (tokenContainsHTMLSpace(token) || tokenContainsHTMLSpace(newToken))
+    if (tokenContainsHTMLSpace(item) || tokenContainsHTMLSpace(replacement))
         return Exception { INVALID_CHARACTER_ERR };
 
     auto& tokens = this->tokens();
-    size_t index = tokens.find(token);
+
+    auto matchesItemOrReplacement = [&](auto& token) {
+        return token == item || token == replacement;
+    };
+
+    size_t index = tokens.findMatching(matchesItemOrReplacement);
     if (index == notFound)
         return { };
 
-    if (tokens.find(newToken) != notFound)
-        tokens.remove(index);
-    else
-        tokens[index] = newToken;
+    tokens[index] = replacement;
+    tokens.removeFirstMatching(matchesItemOrReplacement, index + 1);
+    ASSERT(tokens.find(item) == notFound);
+    ASSERT(tokens.reverseFind(replacement) == index);
 
     updateAssociatedAttributeFromTokens();
 
index 7d10742..c18eca6 100644 (file)
@@ -1,3 +1,15 @@
+2017-04-28  Chris Dumez  <cdumez@apple.com>
+
+        Update DOMTokenList.replace() to match the latest DOM specification
+        https://bugs.webkit.org/show_bug.cgi?id=171388
+
+        Reviewed by Alex Christensen.
+
+        Add API test coverage for new Vector API.
+
+        * TestWebKitAPI/Tests/WTF/Vector.cpp:
+        (TestWebKitAPI::TEST):
+
 2017-04-28  Jonathan Bedard  <jbedard@apple.com>
 
         WebKitTestRunner/DumpRenderTree prevent device from sleeping
index 48235b1..4daba30 100644 (file)
@@ -560,6 +560,21 @@ TEST(WTF_Vector, RemoveAll)
     EXPECT_TRUE(v2 == vExpected);
 }
 
+TEST(WTF_Vector, FindMatching)
+{
+    Vector<int> v;
+    EXPECT_TRUE(v.findMatching([](int) { return false; }) == notFound);
+    EXPECT_TRUE(v.findMatching([](int) { return true; }) == notFound);
+
+    v = {3, 1, 2, 1, 2, 1, 2, 2, 1, 1, 1, 3};
+    EXPECT_TRUE(v.findMatching([](int value) { return value > 3; }) == notFound);
+    EXPECT_TRUE(v.findMatching([](int) { return false; }) == notFound);
+    EXPECT_EQ(0U, v.findMatching([](int) { return true; }));
+    EXPECT_EQ(0U, v.findMatching([](int value) { return value <= 3; }));
+    EXPECT_EQ(1U, v.findMatching([](int value) { return value < 3; }));
+    EXPECT_EQ(2U, v.findMatching([](int value) { return value == 2; }));
+}
+
 TEST(WTF_Vector, RemoveFirstMatching)
 {
     Vector<int> v;
@@ -583,6 +598,16 @@ TEST(WTF_Vector, RemoveFirstMatching)
     EXPECT_TRUE(v.removeFirstMatching([] (int value) { return value > 2; }));
     EXPECT_EQ(9U, v.size());
     EXPECT_TRUE(v == Vector<int>({2, 1, 2, 1, 2, 2, 1, 1, 1}));
+    EXPECT_TRUE(v.removeFirstMatching([] (int value) { return value == 1; }, 1));
+    EXPECT_EQ(8U, v.size());
+    EXPECT_TRUE(v == Vector<int>({2, 2, 1, 2, 2, 1, 1, 1}));
+    EXPECT_TRUE(v.removeFirstMatching([] (int value) { return value == 1; }, 3));
+    EXPECT_EQ(7U, v.size());
+    EXPECT_TRUE(v == Vector<int>({2, 2, 1, 2, 2, 1, 1}));
+    EXPECT_FALSE(v.removeFirstMatching([] (int value) { return value == 1; }, 7));
+    EXPECT_EQ(7U, v.size());
+    EXPECT_FALSE(v.removeFirstMatching([] (int value) { return value == 1; }, 10));
+    EXPECT_EQ(7U, v.size());
 }
 
 TEST(WTF_Vector, RemoveAllMatching)
@@ -612,6 +637,18 @@ TEST(WTF_Vector, RemoveAllMatching)
     EXPECT_TRUE(v == Vector<int>({2, 2, 2, 2}));
     EXPECT_EQ(4U, v.removeAllMatching([] (int value) { return value == 2; }));
     EXPECT_TRUE(v.isEmpty());
+
+    v = {3, 1, 2, 1, 2, 1, 3, 2, 2, 1, 1, 1, 3};
+    EXPECT_EQ(13U, v.size());
+    EXPECT_EQ(0U, v.removeAllMatching([] (int value) { return value > 0; }, 13));
+    EXPECT_EQ(13U, v.size());
+    EXPECT_EQ(0U, v.removeAllMatching([] (int value) { return value > 0; }, 20));
+    EXPECT_EQ(13U, v.size());
+    EXPECT_EQ(5U, v.removeAllMatching([] (int value) { return value > 1; }, 3));
+    EXPECT_EQ(8U, v.size());
+    EXPECT_TRUE(v == Vector<int>({3, 1, 2, 1, 1, 1, 1, 1}));
+    EXPECT_EQ(8U, v.removeAllMatching([] (int value) { return value > 0; }, 0));
+    EXPECT_EQ(0U, v.size());
 }
 
 } // namespace TestWebKitAPI