Source/WebCore/ChangeLog

 12015-08-21 Chris Dumez <cdumez@apple.com>
 2
 3 NodeList should not have a named getter
 4 https://bugs.webkit.org/show_bug.cgi?id=148117
 5
 6 Reviewed by NOBODY (OOPS!).
 7
 8 Drop the named property getter on NodeList to match the specification:
 9 https://dom.spec.whatwg.org/#interface-nodelist
 10
 11 This change could be a bit risky but:
 12 1. Firefox and IE never had this named property getter on NodeList.
 13 2. Chrome successfuly dropped this named property getter in early 2014:
 14 https://src.chromium.org/viewvc/blink?revision=166356&view=revision
 15 3. Our named property getter on NodeList is only partly functional:
 16 It only matches by 'id' attribute, not by 'name' attribute.
 17 4. Our APIs that were wrongly returning a NodeList instead of an
 18 HTMLCollection (getElementsByTagName() / getElementsByClassName())
 19 have been fixed in r188735 and r188809. HTMLCollection does have
 20 a named property getter.
 21 5. This named getter adds code complexity.
 22
 23 * bindings/js/JSNodeListCustom.cpp:
 24 (WebCore::createWrapper): Deleted.
 25 * dom/ChildNodeList.cpp:
 26 (WebCore::ChildNodeList::invalidateCache): Deleted.
 27 * dom/ChildNodeList.h:
 28 * dom/LiveNodeList.cpp:
 29 * dom/LiveNodeList.h:
 30 * dom/NodeList.h:
 31 * dom/NodeList.idl:
 32 * dom/StaticNodeList.cpp:
 33 (WebCore::StaticElementList::length): Deleted.
 34 (WebCore::StaticElementList::item): Deleted.
 35 * dom/StaticNodeList.h:
 36 * html/HTMLCollection.h:
 37
1382015-08-21 Commit Queue <commit-queue@webkit.org>
239
340 Unreviewed, rolling out r188792 and r188803.

Source/WebCore/bindings/js/JSNodeListCustom.cpp

@@bool JSNodeListOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handl
5151 return false;
5252}
5353
54 // FIXME: NodeList should not have a named getter. It currently has one because getElementByTagName()
55 // returns a NodeList instead of an HTMLCollection.
56 bool JSNodeList::nameGetter(ExecState* exec, PropertyName propertyName, JSValue& value)
57 {
58  auto item = impl().namedItem(propertyNameToAtomicString(propertyName));
59  if (!item)
60  return false;
61 
62  value = toJS(exec, globalObject(), item);
63  return true;
64 }
65 
6654JSC::JSValue createWrapper(JSDOMGlobalObject& globalObject, NodeList& nodeList)
6755{
6856 // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.

Source/WebCore/dom/ChildNodeList.cpp

@@void ChildNodeList::collectionTraverseBackward(Node*& current, unsigned count) c
8181 current = current->previousSibling();
8282}
8383
84 Node* ChildNodeList::namedItem(const AtomicString& name) const
85 {
86  // FIXME: According to the spec child node list should not have namedItem().
87  if (m_parent.get().inDocument()) {
88  Element* element = m_parent.get().treeScope().getElementById(name);
89  if (element && element->parentNode() == m_parent.ptr())
90  return element;
91  if (!element || !m_parent.get().treeScope().containsMultipleElementsWithId(name))
92  return nullptr;
93  }
94  for (auto& element : childrenOfType<Element>(m_parent)) {
95  if (element.hasID() && element.idForStyleResolution() == name)
96  return const_cast<Element*>(&element);
97  }
98  return nullptr;
99 }
100 
10184void ChildNodeList::invalidateCache()
10285{
10386 m_indexCache.invalidate(*this);

Source/WebCore/dom/ChildNodeList.h

@@private:
4848
4949 virtual unsigned length() const override { return 0; }
5050 virtual Node* item(unsigned) const override { return nullptr; }
51  virtual Node* namedItem(const AtomicString&) const override { return nullptr; }
5251 virtual size_t memoryCost() const override { return 0; }
5352
5453 virtual bool isEmptyNodeList() const override { return true; }

@@private:
8382
8483 virtual unsigned length() const override;
8584 virtual Node* item(unsigned index) const override;
86  virtual Node* namedItem(const AtomicString&) const override;
8785 virtual size_t memoryCost() const override { return m_indexCache.memoryCost(); }
8886
8987 virtual bool isChildNodeList() const override { return true; }

Source/WebCore/dom/LiveNodeList.cpp

@@ContainerNode& LiveNodeList::rootNode() const
5050 return ownerNode();
5151}
5252
53 Node* LiveNodeList::namedItem(const AtomicString& elementId) const
54 {
55  // FIXME: Why doesn't this look into the name attribute like HTMLCollection::namedItem does?
56  Node& rootNode = this->rootNode();
57 
58  if (rootNode.inDocument()) {
59  Element* element = rootNode.treeScope().getElementById(elementId);
60  if (element && elementMatches(*element) && element->isDescendantOf(&rootNode))
61  return element;
62  if (!element)
63  return nullptr;
64  // In the case of multiple nodes with the same name, just fall through.
65  }
66 
67  if (elementId.isEmpty())
68  return nullptr;
69 
70  unsigned length = this->length();
71  for (unsigned i = 0; i < length; i++) {
72  Node* node = item(i);
73  if (!is<Element>(*node))
74  continue;
75  Element& element = downcast<Element>(*node);
76  // FIXME: This should probably be using getIdAttribute instead of idForStyleResolution.
77  if (element.hasID() && element.idForStyleResolution() == elementId)
78  return node;
79  }
80 
81  return nullptr;
82 }
83 
8453} // namespace WebCore

Source/WebCore/dom/LiveNodeList.h

@@public:
4545 LiveNodeList(ContainerNode& ownerNode, NodeListInvalidationType);
4646 virtual ~LiveNodeList();
4747
48  virtual Node* namedItem(const AtomicString&) const override final;
49 
5048 virtual bool elementMatches(Element&) const = 0;
5149 virtual bool isRootedAtDocument() const = 0;
5250

Source/WebCore/dom/NodeList.h

@@public:
3939 // DOM methods & attributes for NodeList
4040 virtual unsigned length() const = 0;
4141 virtual Node* item(unsigned index) const = 0;
42  virtual Node* namedItem(const AtomicString&) const = 0;
4342
4443 // Other methods (not part of DOM)
4544 virtual bool isLiveNodeList() const { return false; }

Source/WebCore/dom/NodeList.idl

2020
2121[
2222 CustomIsReachable,
23  CustomNamedGetter,
2423 CustomToJSObject,
2524 JSCustomHeader,
2625 SkipVTableValidation,
2726 ReportExtraMemoryCost,
2827] interface NodeList {
29 
3028 getter Node item(unsigned long index);
31 
3229 readonly attribute unsigned long length;
33 
3430};
3531

Source/WebCore/dom/StaticNodeList.cpp

@@Node* StaticNodeList::item(unsigned index) const
4343 return nullptr;
4444}
4545
46 Node* StaticNodeList::namedItem(const AtomicString& elementId) const
47 {
48  if (elementId.isEmpty())
49  return nullptr;
50  for (unsigned i = 0, length = m_nodes.size(); i < length; ++i) {
51  Node& node = const_cast<Node&>(m_nodes[i].get());
52  if (is<Element>(node) && downcast<Element>(node).getIdAttribute() == elementId)
53  return &node;
54  }
55  return nullptr;
56 }
57 
5846unsigned StaticElementList::length() const
5947{
6048 return m_elements.size();

@@Node* StaticElementList::item(unsigned index) const
6755 return nullptr;
6856}
6957
70 Node* StaticElementList::namedItem(const AtomicString& elementId) const
71 {
72  if (elementId.isEmpty())
73  return nullptr;
74  for (unsigned i = 0, length = m_elements.size(); i < length; ++i) {
75  Element& element = const_cast<Element&>(m_elements[i].get());
76  if (element.getIdAttribute() == elementId)
77  return &element;
78  }
79  return nullptr;
80 }
81 
8258} // namespace WebCore

Source/WebCore/dom/StaticNodeList.h

@@public:
5353
5454 virtual unsigned length() const override;
5555 virtual Node* item(unsigned index) const override;
56  virtual Node* namedItem(const AtomicString&) const override;
5756
5857private:
5958 StaticNodeList() { }

@@public:
7776
7877 virtual unsigned length() const override;
7978 virtual Node* item(unsigned index) const override;
80  virtual Node* namedItem(const AtomicString&) const override;
8179
8280private:
8381 StaticElementList()

Source/WebCore/html/HTMLCollection.h

@@public:
6565
6666 // DOM API
6767 virtual Element* item(unsigned index) const override = 0; // Tighten return type from NodeList::item().
68  virtual Element* namedItem(const AtomicString& name) const override = 0; // Tighten return type from NodeList::namedItem().
 68 virtual Element* namedItem(const AtomicString& name) const = 0;
6969 RefPtr<NodeList> tags(const String&);
7070
7171 // Non-DOM API

LayoutTests/ChangeLog

112015-08-21 Chris Dumez <cdumez@apple.com>
22
 3 NodeList should not have a named getter
 4 https://bugs.webkit.org/show_bug.cgi?id=148117
 5
 6 Reviewed by NOBODY (OOPS!).
 7
 8 * fast/dom/childnode-item-after-itemname-expected.txt: Removed.
 9 * fast/dom/childnode-item-after-itemname.html: Removed.
 10 Drop test as it is no longer relevant now that NodeList no longer
 11 has a named property getter.
 12
 132015-08-21 Chris Dumez <cdumez@apple.com>
 14
315 document.getElementsByTagName should return an HTMLCollection
416 https://bugs.webkit.org/show_bug.cgi?id=110611
517

LayoutTests/fast/dom/childnode-item-after-itemname-expected.txt

1 This tests accessing the first element in childNodes after accessing the second element by its id. WebKit should retrieve each element correctly, and you should see 2 1 below:
2 
3 2 1

LayoutTests/fast/dom/childnode-item-after-itemname.html

1 <!DOCTYPE html>
2 <html>
3 <body>
4 <p>This tests accessing the first element in childNodes after accessing the second element by its id.
5 WebKit should retrieve each element correctly, and you should see 2 1 below:</p>
6 <span id="testItem"></span><span id="tests" style="display: none;">1<span id="testItem">2</span></span>
7 <script>
8 
9 if (window.testRunner)
10  testRunner.dumpAsText();
11 
12 var tests = document.querySelector('#tests');
13 document.writeln(tests.childNodes['testItem'].textContent);
14 document.writeln(tests.childNodes[0].textContent);
15 
16 </script>
17 </body>
18 </html>

LayoutTests/fast/dom/named-items-with-symbol-name-expected.txt

@@This tests Symbol property names with a number of named items collections.
33On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
44
55
6 [object NodeList]
7 PASS querySelectorAllList.length === 4 is true
8 PASS querySelectorAllList[Symbol()] === undefined is true
9 PASS querySelectorAllList[Symbol('div')] === undefined is true
10 PASS querySelectorAllList['Symbol(div)'] instanceof HTMLDivElement is true
11 
126[object HTMLCollection]
137PASS getElementsByTagNameList.length === 4 is true
148PASS getElementsByTagNameList[Symbol()] === undefined is true

LayoutTests/fast/dom/named-items-with-symbol-name.html

1515<script>
1616description('This tests Symbol property names with a number of named items collections.');
1717
18 var querySelectorAllList = document.querySelectorAll('div');
19 debug(String(querySelectorAllList));
20 shouldBeTrue("querySelectorAllList.length === 4");
21 shouldBeTrue("querySelectorAllList[Symbol()] === undefined");
22 shouldBeTrue("querySelectorAllList[Symbol('div')] === undefined");
23 shouldBeTrue("querySelectorAllList['Symbol(div)'] instanceof HTMLDivElement");
24 
2518var getElementsByTagNameList = document.getElementsByTagName('div');
2619debug("\n" + String(getElementsByTagNameList));
2720shouldBeTrue("getElementsByTagNameList.length === 4");