Skip to content

Commit ecb1530

Browse files
author
Sam Weinig
committed
[WebIDL/DOM] Remove need for custom bindings for HTMLAllCollection and bring up to spec
https://bugs.webkit.org/show_bug.cgi?id=172095 Reviewed by Darin Adler. LayoutTests/imported/w3c: * web-platform-tests/html/infrastructure/common-dom-interfaces/collections/htmlallcollection-expected.txt: Update results. Source/WebCore: - Adds support for the legacycaller WebIDL special annotation. - Updates implementation of HTMLAllCollection to match the current HTML spec. Test: fast/dom/document-all.html * CMakeLists.txt: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSBindingsAllInOne.cpp: * bindings/js/JSHTMLAllCollectionCustom.cpp: Removed. Removed JSHTMLAllCollectionCustom.cpp * bindings/scripts/CodeGeneratorJS.pm: (GenerateInterface): (AddLegacyCallerOperationIfNeeded): Before code generation, clone all the legacycaller operations and put them in their own set, so they can form an overload set. (AddStringifierOperationIfNeeded): Use IDLParser::cloneType as the FIXME suggested. (GenerateHeader): Group call related functionality together and use new IsCallable predicate. (GenerateOverloadedFunctionOrConstructor): Generalize a little bit to allow the function being overloaded to be an overloaded legacycaller. (GenerateImplementation): Add call to generate the legacycaller code. (GenerateLegacyCallerDefinitions): (GenerateLegacyCallerDefinition): Generate the legacycaller definition, using GenerateArgumentsCountCheck, GenerateParametersCheck and GenerateImplementationFunctionCall to do all the heavy lifting. (IsCallable): Add helper predicate for both custom calls and legacycaller. * bindings/scripts/IDLParser.pm: (cloneType):. (cloneArgument):. (cloneOperation): Add cloning functions for IDLArgument and IDLOperation, and make IDLType's clone feasible for calling outside the package by removing the unneeded self parameter. * bindings/scripts/test/JS/JSTestObj.cpp * bindings/scripts/test/JS/JSTestObj.h * bindings/scripts/test/TestObj.idl: Add testing of legacycaller overloading. * dom/Document.cpp: (WebCore::Document::allFilteredByName): * dom/Document.h: Add new collection access for the HTMLAllNamedSubCollection. * html/CachedHTMLCollection.h: (WebCore::nameShouldBeVisibleInDocumentAll): Update list of tags to match the current spec. * html/CollectionType.h: Add new type for HTMLAllNamedSubCollection. * html/GenericCachedHTMLCollection.cpp: (WebCore::GenericCachedHTMLCollection<traversalType>::elementMatches): Specify that DocumentAllNamedItems does not want the default elementMatches. * html/HTMLAllCollection.cpp: (WebCore::HTMLAllCollection::namedOrIndexedItemOrItems): (WebCore::HTMLAllCollection::namedItemOrItems): (WebCore::HTMLAllNamedSubCollection::~HTMLAllNamedSubCollection): (WebCore::HTMLAllNamedSubCollection::elementMatches): * html/HTMLAllCollection.h: Move implementations from the custom binding, and re-implement to match the spec. Alternate names to item/namedItem were needed to not shadow the existing ones in HTMLCollection. HTMLAllNamedSubCollection is a simple HTMLCollection that matches on a name, following the rules of document.all about which tags can have name attributes. * html/HTMLAllCollection.idl: Remove custom annotations and add legacycaller which is now supported. * html/HTMLCollection.cpp: (WebCore::invalidationTypeExcludingIdAndNameAttributes): (WebCore::HTMLCollection::~HTMLCollection): Add DocumentAllNamedItems. LayoutTests: * fast/dom/collection-null-like-arguments-expected.txt: Update results. * fast/dom/document-all-expected.txt: Added. * fast/dom/document-all.html: Added. New test that covers a bunch of missing coverage. Canonical link: https://commits.webkit.org/189034@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@216851 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 14727bf commit ecb1530

25 files changed

Lines changed: 689 additions & 190 deletions

‎LayoutTests/ChangeLog‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
2017-05-14 Sam Weinig <sam@webkit.org>
2+
3+
[WebIDL/DOM] Remove need for custom bindings for HTMLAllCollection and bring up to spec
4+
https://bugs.webkit.org/show_bug.cgi?id=172095
5+
6+
Reviewed by Darin Adler.
7+
8+
* fast/dom/collection-null-like-arguments-expected.txt:
9+
Update results.
10+
11+
* fast/dom/document-all-expected.txt: Added.
12+
* fast/dom/document-all.html: Added.
13+
New test that covers a bunch of missing coverage.
14+
115
2017-05-14 David Kilzer <ddkilzer@apple.com>
216

317
[iOS/macOS Debug WK2] LayoutTests/imported/w3c/web-platform-tests/webrtc/interfaces.html is a flaky crash due to assertion failure

‎LayoutTests/fast/dom/collection-null-like-arguments-expected.txt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,19 @@ document.all(0): [object HTMLHtmlElement]
77
document.all.item(0): [object HTMLHtmlElement]
88

99
document.all[""]: undefined
10-
document.all(""): undefined
11-
document.all.item(""): undefined
10+
document.all(""): null
11+
document.all.item(""): null
1212

1313
document.all["0"]: [object HTMLHtmlElement]
1414
document.all("0"): [object HTMLHtmlElement]
1515
document.all.item("0"): [object HTMLHtmlElement]
1616

1717
document.all[undefined]: undefined
18-
document.all(undefined): undefined
19-
document.all.item(undefined): undefined
18+
document.all(undefined): null
19+
document.all.item(undefined): null
2020

2121
document.all[null]: undefined
22-
document.all(null): undefined
23-
document.all.item(null): undefined
22+
document.all(null): null
23+
document.all.item(null): null
2424

2525

‎LayoutTests/fast/dom/document-all-expected.txt‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
Tests document.all
2+
3+
4+
PASS document.all's index getter returns all the elements in document order
5+
PASS document.all's index getter returns undefined for indexes greater than length
6+
PASS document.all's index getter returns undefined for indexes less than zero
7+
PASS document.all's name attribute accessing works for a
8+
PASS document.all's name attribute accessing works for applet
9+
PASS document.all's name attribute accessing works for button
10+
PASS document.all's name attribute accessing works for embed
11+
PASS document.all's name attribute accessing works for form
12+
PASS document.all's name attribute accessing works for frame
13+
PASS document.all's name attribute accessing works for frameset
14+
PASS document.all's name attribute accessing works for iframe
15+
PASS document.all's name attribute accessing works for img
16+
PASS document.all's name attribute accessing works for input
17+
PASS document.all's name attribute accessing works for map
18+
PASS document.all's name attribute accessing works for meta
19+
PASS document.all's name attribute accessing works for object
20+
PASS document.all's name attribute accessing works for select
21+
PASS document.all's name attribute accessing works for textarea
22+
PASS document.all's name attribute accessing doesn't work for div
23+
PASS document.all's name attribute accessing doesn't work for span
24+
PASS document.all's id attribute accessing works for a
25+
PASS document.all's id attribute accessing works for applet
26+
PASS document.all's id attribute accessing works for button
27+
PASS document.all's id attribute accessing works for embed
28+
PASS document.all's id attribute accessing works for form
29+
PASS document.all's id attribute accessing works for frame
30+
PASS document.all's id attribute accessing works for frameset
31+
PASS document.all's id attribute accessing works for iframe
32+
PASS document.all's id attribute accessing works for img
33+
PASS document.all's id attribute accessing works for input
34+
PASS document.all's id attribute accessing works for map
35+
PASS document.all's id attribute accessing works for meta
36+
PASS document.all's id attribute accessing works for object
37+
PASS document.all's id attribute accessing works for select
38+
PASS document.all's id attribute accessing works for textarea
39+
PASS document.all's id attribute accessing works for div
40+
PASS document.all's id attribute accessing works for span
41+
PASS document.all will return a sub-collection if the name/id is found more than once
42+
PASS document.all will return a sub-collection if the name/id is found more than once, but still adheres to the rules about what tags can have names
43+
PASS document.all will return a sub-collection if the name/id is found more than once, that is live
44+
PASS document.all's namedItem function requires passing an argument
45+
PASS document.all's item function and legacy caller functionality allow passing no arguments
46+
PASS document.all's item function and legacy caller functionality will try to convert their DOMString argument to an index before name lookup
47+

‎LayoutTests/fast/dom/document-all.html‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
<!DOCTYPE html>
2+
<head>
3+
<title>document.all</title>
4+
<body>
5+
<p>Tests document.all</p>
6+
<script src="../../resources/testharness.js"></script>
7+
<script src="../../resources/testharnessreport.js"></script>
8+
<script>
9+
10+
let validNamedElementTags = ['a', 'applet', 'button', 'embed', 'form', 'frame', 'frameset', 'iframe', 'img', 'input', 'map', 'meta', 'object', 'select', 'textarea'];
11+
let invalidNamedElementTags = ['div', 'span'];
12+
let validAnInvalidNamedElementTags = validNamedElementTags.concat(invalidNamedElementTags);
13+
14+
let playground = document.createElement('div');
15+
document.body.append(playground);
16+
17+
test(function() {
18+
assert_equals(document.all[0], document.documentElement);
19+
assert_equals(document.all[1], document.head);
20+
assert_equals(document.all[2], document.head.firstElementChild);
21+
}, "document.all's index getter returns all the elements in document order");
22+
23+
test(function() {
24+
assert_equals(document.all[document.all.length + 1], undefined);
25+
}, "document.all's index getter returns undefined for indexes greater than length");
26+
27+
test(function() {
28+
assert_equals(document.all[-1], undefined);
29+
}, "document.all's index getter returns undefined for indexes less than zero");
30+
31+
for (let elementTag of validNamedElementTags) {
32+
test(function() {
33+
let element = document.createElement(elementTag);
34+
element.setAttribute("name", "testName");
35+
playground.append(element);
36+
37+
assert_equals(document.all.namedItem('testName'), element);
38+
assert_equals(document.all['testName'], element);
39+
assert_equals(document.all.item('testName'), element);
40+
assert_equals(document.all('testName'), element);
41+
42+
element.remove();
43+
}, "document.all's name attribute accessing works for " + elementTag);
44+
}
45+
46+
for (let elementTag of invalidNamedElementTags) {
47+
test(function() {
48+
let element = document.createElement(elementTag);
49+
element.setAttribute("name", "testName");
50+
playground.append(element);
51+
52+
assert_equals(document.all.namedItem('testName'), null);
53+
assert_equals(document.all['testName'], undefined);
54+
assert_equals(document.all.item('testName'), null);
55+
assert_equals(document.all('testName'), null);
56+
57+
element.remove();
58+
}, "document.all's name attribute accessing doesn't work for " + elementTag);
59+
}
60+
61+
for (let elementTag of validAnInvalidNamedElementTags) {
62+
test(function() {
63+
let element = document.createElement(elementTag);
64+
element.setAttribute("id", "testId");
65+
playground.append(element);
66+
67+
assert_equals(document.all.namedItem('testId'), element);
68+
assert_equals(document.all['testId'], element);
69+
assert_equals(document.all.item('testId'), element);
70+
assert_equals(document.all('testId'), element);
71+
72+
element.remove();
73+
}, "document.all's id attribute accessing works for " + elementTag);
74+
}
75+
76+
test(function() {
77+
let element1 = document.createElement('button');
78+
element1.setAttribute("name", "test");
79+
playground.append(element1);
80+
81+
let element2 = document.createElement('span');
82+
element2.setAttribute("id", "test");
83+
playground.append(element2);
84+
85+
assert_equals(document.all.namedItem('test').length, 2);
86+
assert_equals(document.all['test'].length, 2);
87+
assert_equals(document.all.item('test').length, 2);
88+
assert_equals(document.all('test').length, 2);
89+
assert_class_string(document.all.namedItem('test'), "HTMLCollection");
90+
assert_class_string(document.all['test'], "HTMLCollection");
91+
assert_class_string(document.all.item('test'), "HTMLCollection");
92+
assert_class_string(document.all('test'), "HTMLCollection");
93+
94+
element1.remove();
95+
element2.remove();
96+
}, "document.all will return a sub-collection if the name/id is found more than once");
97+
98+
test(function() {
99+
let element1 = document.createElement('button');
100+
element1.setAttribute("name", "test");
101+
playground.append(element1);
102+
103+
let element2 = document.createElement('span');
104+
element2.setAttribute("id", "test");
105+
playground.append(element2);
106+
107+
let element3 = document.createElement('span');
108+
element3.setAttribute("name", "test");
109+
playground.append(element2);
110+
111+
assert_equals(document.all.namedItem('test').length, 2);
112+
assert_equals(document.all['test'].length, 2);
113+
assert_equals(document.all.item('test').length, 2);
114+
assert_equals(document.all('test').length, 2);
115+
assert_equals(document.all.namedItem('test')[0], element1);
116+
assert_equals(document.all.namedItem('test')[1], element2);
117+
assert_equals(document.all['test'][0], element1);
118+
assert_equals(document.all['test'][1], element2);
119+
assert_equals(document.all.item('test')[0], element1);
120+
assert_equals(document.all.item('test')[1], element2);
121+
assert_equals(document.all('test')[0], element1);
122+
assert_equals(document.all('test')[1], element2);
123+
124+
element1.remove();
125+
element2.remove();
126+
element3.remove();
127+
}, "document.all will return a sub-collection if the name/id is found more than once, but still adheres to the rules about what tags can have names");
128+
129+
test(function() {
130+
let element1 = document.createElement('button');
131+
element1.setAttribute("name", "test");
132+
playground.append(element1);
133+
134+
let element2 = document.createElement('span');
135+
element2.setAttribute("id", "test");
136+
playground.append(element2);
137+
138+
let collection = document.all.namedItem('test');
139+
assert_equals(collection.length, 2);
140+
141+
let element3 = document.createElement('img');
142+
element3.setAttribute("name", "test");
143+
playground.append(element3);
144+
145+
assert_equals(collection.length, 3);
146+
147+
element1.remove();
148+
element2.remove();
149+
150+
assert_equals(collection.length, 1);
151+
152+
element3.remove();
153+
}, "document.all will return a sub-collection if the name/id is found more than once, that is live");
154+
155+
test(function() {
156+
assert_throws(new TypeError(), function() { document.all.namedItem() });
157+
}, "document.all's namedItem function requires passing an argument");
158+
159+
test(function() {
160+
assert_equals(document.all.item(), null);
161+
assert_equals(document.all(), null);
162+
}, "document.all's item function and legacy caller functionality allow passing no arguments");
163+
164+
test(function() {
165+
let element = document.createElement('button');
166+
element.setAttribute("name", "0");
167+
playground.append(element);
168+
169+
assert_equals(document.all.item('0'), document.documentElement);
170+
assert_equals(document.all('0'), document.documentElement);
171+
assert_equals(document.all.namedItem('0'), element);
172+
173+
element.remove();
174+
}, "document.all's item function and legacy caller functionality will try to convert their DOMString argument to an index before name lookup");
175+
176+
playground.remove();
177+
178+
</script>
179+
</body>

‎LayoutTests/imported/w3c/ChangeLog‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
2017-05-14 Sam Weinig <sam@webkit.org>
2+
3+
[WebIDL/DOM] Remove need for custom bindings for HTMLAllCollection and bring up to spec
4+
https://bugs.webkit.org/show_bug.cgi?id=172095
5+
6+
Reviewed by Darin Adler.
7+
8+
* web-platform-tests/html/infrastructure/common-dom-interfaces/collections/htmlallcollection-expected.txt:
9+
Update results.
10+
111
2017-05-12 Romain Bellessort <romain.bellessort@crf.canon.fr>
212

313
[Readable Streams API] Add ReadableStreamBYOBReader closed getter

‎LayoutTests/imported/w3c/web-platform-tests/html/infrastructure/common-dom-interfaces/collections/htmlallcollection-expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,5 @@ PASS Test lookup IMG in collection using []
1010
PASS Test lookup IMG in collection using .
1111
PASS Test lookup tags in collection using .
1212
PASS Should find root element too
13-
FAIL Should find both anchors and produce a list undefined is not an object (evaluating 'document.all["foo"].length')
13+
PASS Should find both anchors and produce a list
1414

‎Source/WebCore/CMakeLists.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1168,7 +1168,6 @@ set(WebCore_SOURCES
11681168
bindings/js/JSEventListener.cpp
11691169
bindings/js/JSEventTargetCustom.cpp
11701170
bindings/js/JSExceptionBase.cpp
1171-
bindings/js/JSHTMLAllCollectionCustom.cpp
11721171
bindings/js/JSHTMLAppletElementCustom.cpp
11731172
bindings/js/JSHTMLCanvasElementCustom.cpp
11741173
bindings/js/JSHTMLCollectionCustom.cpp

‎Source/WebCore/ChangeLog‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,97 @@
1+
2017-05-14 Sam Weinig <sam@webkit.org>
2+
3+
[WebIDL/DOM] Remove need for custom bindings for HTMLAllCollection and bring up to spec
4+
https://bugs.webkit.org/show_bug.cgi?id=172095
5+
6+
Reviewed by Darin Adler.
7+
8+
- Adds support for the legacycaller WebIDL special annotation.
9+
- Updates implementation of HTMLAllCollection to match the current HTML spec.
10+
11+
Test: fast/dom/document-all.html
12+
13+
* CMakeLists.txt:
14+
* WebCore.xcodeproj/project.pbxproj:
15+
* bindings/js/JSBindingsAllInOne.cpp:
16+
* bindings/js/JSHTMLAllCollectionCustom.cpp: Removed.
17+
Removed JSHTMLAllCollectionCustom.cpp
18+
19+
* bindings/scripts/CodeGeneratorJS.pm:
20+
(GenerateInterface):
21+
(AddLegacyCallerOperationIfNeeded):
22+
Before code generation, clone all the legacycaller operations and put them
23+
in their own set, so they can form an overload set.
24+
25+
(AddStringifierOperationIfNeeded):
26+
Use IDLParser::cloneType as the FIXME suggested.
27+
28+
(GenerateHeader):
29+
Group call related functionality together and use new IsCallable predicate.
30+
31+
(GenerateOverloadedFunctionOrConstructor):
32+
Generalize a little bit to allow the function being overloaded to be an overloaded legacycaller.
33+
34+
(GenerateImplementation):
35+
Add call to generate the legacycaller code.
36+
37+
(GenerateLegacyCallerDefinitions):
38+
(GenerateLegacyCallerDefinition):
39+
Generate the legacycaller definition, using GenerateArgumentsCountCheck, GenerateParametersCheck
40+
and GenerateImplementationFunctionCall to do all the heavy lifting.
41+
42+
(IsCallable):
43+
Add helper predicate for both custom calls and legacycaller.
44+
45+
* bindings/scripts/IDLParser.pm:
46+
(cloneType):.
47+
(cloneArgument):.
48+
(cloneOperation):
49+
Add cloning functions for IDLArgument and IDLOperation, and make IDLType's
50+
clone feasible for calling outside the package by removing the unneeded
51+
self parameter.
52+
53+
* bindings/scripts/test/JS/JSTestObj.cpp
54+
* bindings/scripts/test/JS/JSTestObj.h
55+
* bindings/scripts/test/TestObj.idl:
56+
Add testing of legacycaller overloading.
57+
58+
* dom/Document.cpp:
59+
(WebCore::Document::allFilteredByName):
60+
* dom/Document.h:
61+
Add new collection access for the HTMLAllNamedSubCollection.
62+
63+
* html/CachedHTMLCollection.h:
64+
(WebCore::nameShouldBeVisibleInDocumentAll):
65+
Update list of tags to match the current spec.
66+
67+
* html/CollectionType.h:
68+
Add new type for HTMLAllNamedSubCollection.
69+
70+
* html/GenericCachedHTMLCollection.cpp:
71+
(WebCore::GenericCachedHTMLCollection<traversalType>::elementMatches):
72+
Specify that DocumentAllNamedItems does not want
73+
the default elementMatches.
74+
75+
* html/HTMLAllCollection.cpp:
76+
(WebCore::HTMLAllCollection::namedOrIndexedItemOrItems):
77+
(WebCore::HTMLAllCollection::namedItemOrItems):
78+
(WebCore::HTMLAllNamedSubCollection::~HTMLAllNamedSubCollection):
79+
(WebCore::HTMLAllNamedSubCollection::elementMatches):
80+
* html/HTMLAllCollection.h:
81+
Move implementations from the custom binding, and re-implement to
82+
match the spec. Alternate names to item/namedItem were needed to not
83+
shadow the existing ones in HTMLCollection. HTMLAllNamedSubCollection
84+
is a simple HTMLCollection that matches on a name, following the rules
85+
of document.all about which tags can have name attributes.
86+
87+
* html/HTMLAllCollection.idl:
88+
Remove custom annotations and add legacycaller which is now supported.
89+
90+
* html/HTMLCollection.cpp:
91+
(WebCore::invalidationTypeExcludingIdAndNameAttributes):
92+
(WebCore::HTMLCollection::~HTMLCollection):
93+
Add DocumentAllNamedItems.
94+
195
2017-05-14 Zalan Bujtas <zalan@apple.com>
296

397
Remove unused lambda in TextFragmentIterator::TextFragment::split() and cleanup dependencies.

0 commit comments

Comments
 (0)