Diff
Modified: trunk/LayoutTests/ChangeLog (188828 => 188829)
--- trunk/LayoutTests/ChangeLog 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/LayoutTests/ChangeLog 2015-08-22 22:29:26 UTC (rev 188829)
@@ -1,3 +1,15 @@
+2015-08-22 Chris Dumez <[email protected]>
+
+ NodeList should not have a named getter
+ https://bugs.webkit.org/show_bug.cgi?id=148117
+
+ Reviewed by Darin Adler.
+
+ * fast/dom/childnode-item-after-itemname-expected.txt: Removed.
+ * fast/dom/childnode-item-after-itemname.html: Removed.
+ Drop test as it is no longer relevant now that NodeList no longer
+ has a named property getter.
+
2015-08-22 Alexey Proskuryakov <[email protected]>
Tweak a test that became flaky on some machines after r188793.
Deleted: trunk/LayoutTests/fast/dom/childnode-item-after-itemname-expected.txt (188828 => 188829)
--- trunk/LayoutTests/fast/dom/childnode-item-after-itemname-expected.txt 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/LayoutTests/fast/dom/childnode-item-after-itemname-expected.txt 2015-08-22 22:29:26 UTC (rev 188829)
@@ -1,3 +0,0 @@
-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 1
Deleted: trunk/LayoutTests/fast/dom/childnode-item-after-itemname.html (188828 => 188829)
--- trunk/LayoutTests/fast/dom/childnode-item-after-itemname.html 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/LayoutTests/fast/dom/childnode-item-after-itemname.html 2015-08-22 22:29:26 UTC (rev 188829)
@@ -1,18 +0,0 @@
-<!DOCTYPE html>
-<html>
-<body>
-<p>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:</p>
-<span id="testItem"></span><span id="tests" style="display: none;">1<span id="testItem">2</span></span>
-<script>
-
-if (window.testRunner)
- testRunner.dumpAsText();
-
-var tests = document.querySelector('#tests');
-document.writeln(tests.childNodes['testItem'].textContent);
-document.writeln(tests.childNodes[0].textContent);
-
-</script>
-</body>
-</html>
Modified: trunk/LayoutTests/fast/dom/named-items-with-symbol-name-expected.txt (188828 => 188829)
--- trunk/LayoutTests/fast/dom/named-items-with-symbol-name-expected.txt 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/LayoutTests/fast/dom/named-items-with-symbol-name-expected.txt 2015-08-22 22:29:26 UTC (rev 188829)
@@ -3,11 +3,6 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-[object NodeList]
-PASS querySelectorAllList.length === 4 is true
-PASS querySelectorAllList[Symbol()] === undefined is true
-PASS querySelectorAllList[Symbol('div')] === undefined is true
-PASS querySelectorAllList['Symbol(div)'] instanceof HTMLDivElement is true
[object HTMLCollection]
PASS getElementsByTagNameList.length === 4 is true
Modified: trunk/LayoutTests/fast/dom/named-items-with-symbol-name.html (188828 => 188829)
--- trunk/LayoutTests/fast/dom/named-items-with-symbol-name.html 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/LayoutTests/fast/dom/named-items-with-symbol-name.html 2015-08-22 22:29:26 UTC (rev 188829)
@@ -15,13 +15,6 @@
<script>
description('This tests Symbol property names with a number of named items collections.');
-var querySelectorAllList = document.querySelectorAll('div');
-debug(String(querySelectorAllList));
-shouldBeTrue("querySelectorAllList.length === 4");
-shouldBeTrue("querySelectorAllList[Symbol()] === undefined");
-shouldBeTrue("querySelectorAllList[Symbol('div')] === undefined");
-shouldBeTrue("querySelectorAllList['Symbol(div)'] instanceof HTMLDivElement");
-
var getElementsByTagNameList = document.getElementsByTagName('div');
debug("\n" + String(getElementsByTagNameList));
shouldBeTrue("getElementsByTagNameList.length === 4");
Modified: trunk/Source/WebCore/ChangeLog (188828 => 188829)
--- trunk/Source/WebCore/ChangeLog 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/ChangeLog 2015-08-22 22:29:26 UTC (rev 188829)
@@ -1,3 +1,40 @@
+2015-08-22 Chris Dumez <[email protected]>
+
+ NodeList should not have a named getter
+ https://bugs.webkit.org/show_bug.cgi?id=148117
+
+ Reviewed by Darin Adler.
+
+ Drop the named property getter on NodeList to match the specification:
+ https://dom.spec.whatwg.org/#interface-nodelist
+
+ This change could be a bit risky but:
+ 1. Firefox and IE never had this named property getter on NodeList.
+ 2. Chrome successfuly dropped this named property getter in early 2014:
+ https://src.chromium.org/viewvc/blink?revision=166356&view=revision
+ 3. Our named property getter on NodeList is only partly functional:
+ It only matches by 'id' attribute, not by 'name' attribute.
+ 4. Our APIs that were wrongly returning a NodeList instead of an
+ HTMLCollection (getElementsByTagName() / getElementsByClassName())
+ have been fixed in r188735 and r188809. HTMLCollection does have
+ a named property getter.
+ 5. This named getter adds code complexity.
+
+ * bindings/js/JSNodeListCustom.cpp:
+ (WebCore::createWrapper): Deleted.
+ * dom/ChildNodeList.cpp:
+ (WebCore::ChildNodeList::invalidateCache): Deleted.
+ * dom/ChildNodeList.h:
+ * dom/LiveNodeList.cpp:
+ * dom/LiveNodeList.h:
+ * dom/NodeList.h:
+ * dom/NodeList.idl:
+ * dom/StaticNodeList.cpp:
+ (WebCore::StaticElementList::length): Deleted.
+ (WebCore::StaticElementList::item): Deleted.
+ * dom/StaticNodeList.h:
+ * html/HTMLCollection.h:
+
2015-08-22 Andreas Kling <[email protected]>
[JSC] Static hash tables should be 100% compile-time constant.
Modified: trunk/Source/WebCore/bindings/js/JSNodeListCustom.cpp (188828 => 188829)
--- trunk/Source/WebCore/bindings/js/JSNodeListCustom.cpp 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/bindings/js/JSNodeListCustom.cpp 2015-08-22 22:29:26 UTC (rev 188829)
@@ -51,18 +51,6 @@
return false;
}
-// FIXME: NodeList should not have a named getter. It currently has one because getElementByTagName()
-// returns a NodeList instead of an HTMLCollection.
-bool JSNodeList::nameGetter(ExecState* exec, PropertyName propertyName, JSValue& value)
-{
- auto item = impl().namedItem(propertyNameToAtomicString(propertyName));
- if (!item)
- return false;
-
- value = toJS(exec, globalObject(), item);
- return true;
-}
-
JSC::JSValue createWrapper(JSDOMGlobalObject& globalObject, NodeList& nodeList)
{
// FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
Modified: trunk/Source/WebCore/dom/ChildNodeList.cpp (188828 => 188829)
--- trunk/Source/WebCore/dom/ChildNodeList.cpp 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/dom/ChildNodeList.cpp 2015-08-22 22:29:26 UTC (rev 188829)
@@ -81,23 +81,6 @@
current = current->previousSibling();
}
-Node* ChildNodeList::namedItem(const AtomicString& name) const
-{
- // FIXME: According to the spec child node list should not have namedItem().
- if (m_parent.get().inDocument()) {
- Element* element = m_parent.get().treeScope().getElementById(name);
- if (element && element->parentNode() == m_parent.ptr())
- return element;
- if (!element || !m_parent.get().treeScope().containsMultipleElementsWithId(name))
- return nullptr;
- }
- for (auto& element : childrenOfType<Element>(m_parent)) {
- if (element.hasID() && element.idForStyleResolution() == name)
- return const_cast<Element*>(&element);
- }
- return nullptr;
-}
-
void ChildNodeList::invalidateCache()
{
m_indexCache.invalidate(*this);
Modified: trunk/Source/WebCore/dom/ChildNodeList.h (188828 => 188829)
--- trunk/Source/WebCore/dom/ChildNodeList.h 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/dom/ChildNodeList.h 2015-08-22 22:29:26 UTC (rev 188829)
@@ -48,7 +48,6 @@
virtual unsigned length() const override { return 0; }
virtual Node* item(unsigned) const override { return nullptr; }
- virtual Node* namedItem(const AtomicString&) const override { return nullptr; }
virtual size_t memoryCost() const override { return 0; }
virtual bool isEmptyNodeList() const override { return true; }
@@ -83,7 +82,6 @@
virtual unsigned length() const override;
virtual Node* item(unsigned index) const override;
- virtual Node* namedItem(const AtomicString&) const override;
virtual size_t memoryCost() const override { return m_indexCache.memoryCost(); }
virtual bool isChildNodeList() const override { return true; }
Modified: trunk/Source/WebCore/dom/LiveNodeList.cpp (188828 => 188829)
--- trunk/Source/WebCore/dom/LiveNodeList.cpp 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/dom/LiveNodeList.cpp 2015-08-22 22:29:26 UTC (rev 188829)
@@ -50,35 +50,4 @@
return ownerNode();
}
-Node* LiveNodeList::namedItem(const AtomicString& elementId) const
-{
- // FIXME: Why doesn't this look into the name attribute like HTMLCollection::namedItem does?
- Node& rootNode = this->rootNode();
-
- if (rootNode.inDocument()) {
- Element* element = rootNode.treeScope().getElementById(elementId);
- if (element && elementMatches(*element) && element->isDescendantOf(&rootNode))
- return element;
- if (!element)
- return nullptr;
- // In the case of multiple nodes with the same name, just fall through.
- }
-
- if (elementId.isEmpty())
- return nullptr;
-
- unsigned length = this->length();
- for (unsigned i = 0; i < length; i++) {
- Node* node = item(i);
- if (!is<Element>(*node))
- continue;
- Element& element = downcast<Element>(*node);
- // FIXME: This should probably be using getIdAttribute instead of idForStyleResolution.
- if (element.hasID() && element.idForStyleResolution() == elementId)
- return node;
- }
-
- return nullptr;
-}
-
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/LiveNodeList.h (188828 => 188829)
--- trunk/Source/WebCore/dom/LiveNodeList.h 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/dom/LiveNodeList.h 2015-08-22 22:29:26 UTC (rev 188829)
@@ -45,8 +45,6 @@
LiveNodeList(ContainerNode& ownerNode, NodeListInvalidationType);
virtual ~LiveNodeList();
- virtual Node* namedItem(const AtomicString&) const override final;
-
virtual bool elementMatches(Element&) const = 0;
virtual bool isRootedAtDocument() const = 0;
Modified: trunk/Source/WebCore/dom/NodeList.h (188828 => 188829)
--- trunk/Source/WebCore/dom/NodeList.h 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/dom/NodeList.h 2015-08-22 22:29:26 UTC (rev 188829)
@@ -39,7 +39,6 @@
// DOM methods & attributes for NodeList
virtual unsigned length() const = 0;
virtual Node* item(unsigned index) const = 0;
- virtual Node* namedItem(const AtomicString&) const = 0;
// Other methods (not part of DOM)
virtual bool isLiveNodeList() const { return false; }
Modified: trunk/Source/WebCore/dom/NodeList.idl (188828 => 188829)
--- trunk/Source/WebCore/dom/NodeList.idl 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/dom/NodeList.idl 2015-08-22 22:29:26 UTC (rev 188829)
@@ -20,16 +20,12 @@
[
CustomIsReachable,
- CustomNamedGetter,
CustomToJSObject,
JSCustomHeader,
SkipVTableValidation,
ReportExtraMemoryCost,
] interface NodeList {
-
getter Node item(unsigned long index);
-
readonly attribute unsigned long length;
-
};
Modified: trunk/Source/WebCore/dom/StaticNodeList.cpp (188828 => 188829)
--- trunk/Source/WebCore/dom/StaticNodeList.cpp 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/dom/StaticNodeList.cpp 2015-08-22 22:29:26 UTC (rev 188829)
@@ -43,18 +43,6 @@
return nullptr;
}
-Node* StaticNodeList::namedItem(const AtomicString& elementId) const
-{
- if (elementId.isEmpty())
- return nullptr;
- for (unsigned i = 0, length = m_nodes.size(); i < length; ++i) {
- Node& node = const_cast<Node&>(m_nodes[i].get());
- if (is<Element>(node) && downcast<Element>(node).getIdAttribute() == elementId)
- return &node;
- }
- return nullptr;
-}
-
unsigned StaticElementList::length() const
{
return m_elements.size();
@@ -67,16 +55,4 @@
return nullptr;
}
-Node* StaticElementList::namedItem(const AtomicString& elementId) const
-{
- if (elementId.isEmpty())
- return nullptr;
- for (unsigned i = 0, length = m_elements.size(); i < length; ++i) {
- Element& element = const_cast<Element&>(m_elements[i].get());
- if (element.getIdAttribute() == elementId)
- return &element;
- }
- return nullptr;
-}
-
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/StaticNodeList.h (188828 => 188829)
--- trunk/Source/WebCore/dom/StaticNodeList.h 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/dom/StaticNodeList.h 2015-08-22 22:29:26 UTC (rev 188829)
@@ -53,7 +53,6 @@
virtual unsigned length() const override;
virtual Node* item(unsigned index) const override;
- virtual Node* namedItem(const AtomicString&) const override;
private:
StaticNodeList() { }
@@ -77,7 +76,6 @@
virtual unsigned length() const override;
virtual Node* item(unsigned index) const override;
- virtual Node* namedItem(const AtomicString&) const override;
private:
StaticElementList()
Modified: trunk/Source/WebCore/html/HTMLCollection.h (188828 => 188829)
--- trunk/Source/WebCore/html/HTMLCollection.h 2015-08-22 22:20:46 UTC (rev 188828)
+++ trunk/Source/WebCore/html/HTMLCollection.h 2015-08-22 22:29:26 UTC (rev 188829)
@@ -65,7 +65,7 @@
// DOM API
virtual Element* item(unsigned index) const override = 0; // Tighten return type from NodeList::item().
- virtual Element* namedItem(const AtomicString& name) const override = 0; // Tighten return type from NodeList::namedItem().
+ virtual Element* namedItem(const AtomicString& name) const = 0;
RefPtr<NodeList> tags(const String&);
// Non-DOM API
