Skip to content

Commit 924e4d2

Browse files
lpasAtkinsSJ
lpas
authored and
AtkinsSJ
committed
LibWeb: Add sorting to Table row HTMLCollection
Add an optional sorting function to HTMLCollection. We use insertion_sort as a stable sorting algorithm so tree order can be maintained.
1 parent 63503c1 commit 924e4d2

File tree

5 files changed

+287
-17
lines changed

5 files changed

+287
-17
lines changed

‎Libraries/LibWeb/DOM/HTMLCollection.cpp‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* SPDX-License-Identifier: BSD-2-Clause
66
*/
77

8+
#include <AK/InsertionSort.h>
89
#include <LibWeb/Bindings/HTMLCollectionPrototype.h>
910
#include <LibWeb/Bindings/Intrinsics.h>
1011
#include <LibWeb/DOM/Document.h>
@@ -17,15 +18,16 @@ namespace Web::DOM {
1718

1819
GC_DEFINE_ALLOCATOR(HTMLCollection);
1920

20-
GC::Ref<HTMLCollection> HTMLCollection::create(ParentNode& root, Scope scope, Function<bool(Element const&)> filter)
21+
GC::Ref<HTMLCollection> HTMLCollection::create(ParentNode& root, Scope scope, Function<bool(Element const&)> filter, Function<bool(Element const&, Element const&)> sort)
2122
{
22-
return root.realm().create<HTMLCollection>(root, scope, move(filter));
23+
return root.realm().create<HTMLCollection>(root, scope, move(filter), move(sort));
2324
}
2425

25-
HTMLCollection::HTMLCollection(ParentNode& root, Scope scope, Function<bool(Element const&)> filter)
26+
HTMLCollection::HTMLCollection(ParentNode& root, Scope scope, Function<bool(Element const&)> filter, Function<bool(Element const&, Element const&)> sort)
2627
: PlatformObject(root.realm())
2728
, m_root(root)
2829
, m_filter(move(filter))
30+
, m_sort(move(sort))
2931
, m_scope(scope)
3032
{
3133
m_legacy_platform_object_flags = LegacyPlatformObjectFlags {
@@ -95,6 +97,13 @@ void HTMLCollection::update_cache_if_needed() const
9597
return IterationDecision::Continue;
9698
});
9799
}
100+
101+
if (m_sort) {
102+
insertion_sort(m_cached_elements, [this](auto const& a, auto const& b) {
103+
return this->m_sort(a, b);
104+
});
105+
}
106+
98107
m_cached_dom_tree_version = root()->document().dom_tree_version();
99108
}
100109

‎Libraries/LibWeb/DOM/HTMLCollection.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class HTMLCollection : public Bindings::PlatformObject {
3030
Children,
3131
Descendants,
3232
};
33-
[[nodiscard]] static GC::Ref<HTMLCollection> create(ParentNode& root, Scope, ESCAPING Function<bool(Element const&)> filter);
33+
[[nodiscard]] static GC::Ref<HTMLCollection> create(ParentNode& root, Scope, ESCAPING Function<bool(Element const&)> filter, ESCAPING Function<bool(Element const&, Element const&)> sort = nullptr);
3434

3535
virtual ~HTMLCollection() override;
3636

@@ -46,7 +46,7 @@ class HTMLCollection : public Bindings::PlatformObject {
4646
virtual bool is_supported_property_name(FlyString const&) const override;
4747

4848
protected:
49-
HTMLCollection(ParentNode& root, Scope, ESCAPING Function<bool(Element const&)> filter);
49+
HTMLCollection(ParentNode& root, Scope, ESCAPING Function<bool(Element const&)> filter, ESCAPING Function<bool(Element const&, Element const&)> sort = nullptr);
5050

5151
virtual void initialize(JS::Realm&) override;
5252

@@ -65,6 +65,7 @@ class HTMLCollection : public Bindings::PlatformObject {
6565

6666
GC::Ref<ParentNode> m_root;
6767
Function<bool(Element const&)> m_filter;
68+
Function<bool(Element const&, Element const&)> m_sort;
6869

6970
Scope m_scope { Scope::Descendants };
7071
};

‎Libraries/LibWeb/HTML/HTMLTableElement.cpp‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -398,14 +398,14 @@ GC::Ref<HTMLTableSectionElement> HTMLTableElement::create_t_body()
398398
GC::Ref<DOM::HTMLCollection> HTMLTableElement::rows()
399399
{
400400
HTMLTableElement* table_node = this;
401-
// FIXME: The elements in the collection must be ordered such that those elements whose parent is a thead are
402-
// included first, in tree order, followed by those elements whose parent is either a table or tbody
403-
// element, again in tree order, followed finally by those elements whose parent is a tfoot element,
404-
// still in tree order.
405-
// How do you sort HTMLCollection?
406-
401+
// The elements in the collection must be ordered such that those elements whose parent is a thead are included
402+
// first, in tree order, followed by those elements whose parent is either a table or tbody element, again in tree
403+
// order, followed finally by those elements whose parent is a tfoot element, still in tree order.
407404
if (!m_rows) {
408-
m_rows = DOM::HTMLCollection::create(*this, DOM::HTMLCollection::Scope::Descendants, [table_node](DOM::Element const& element) {
405+
m_rows = DOM::HTMLCollection::create(
406+
*this,
407+
DOM::HTMLCollection::Scope::Descendants,
408+
[table_node](DOM::Element const& element) {
409409
// Only match TR elements which are:
410410
// * children of the table element
411411
// * children of the thead, tbody, or tfoot elements that are themselves children of the table element
@@ -415,13 +415,29 @@ GC::Ref<DOM::HTMLCollection> HTMLTableElement::rows()
415415
if (element.parent_element() == table_node)
416416
return true;
417417

418-
if (element.parent_element() && (element.parent_element()->local_name() == TagNames::thead || element.parent_element()->local_name() == TagNames::tbody || element.parent_element()->local_name() == TagNames::tfoot)
419-
&& element.parent()->parent() == table_node) {
418+
if (element.parent_element() && element.parent_element()->local_name().is_one_of(TagNames::thead, TagNames::tbody, TagNames::tfoot) && element.parent()->parent() == table_node)
420419
return true;
421-
}
422420

423-
return false;
424-
});
421+
return false; },
422+
[](Element const& a, Element const& b) -> bool {
423+
auto static sort_priority = [](Element const& element) {
424+
auto const& parent_tag = element.parent_element()->local_name();
425+
if (parent_tag == TagNames::thead)
426+
return 1;
427+
if (parent_tag == TagNames::table || parent_tag == TagNames::tbody)
428+
return 2;
429+
if (parent_tag == TagNames::tfoot)
430+
return 3;
431+
VERIFY_NOT_REACHED();
432+
};
433+
434+
auto prio_a = sort_priority(a);
435+
auto prio_b = sort_priority(b);
436+
437+
if (prio_a != prio_b)
438+
return prio_a < prio_b;
439+
440+
return false; });
425441
}
426442
return *m_rows;
427443
}

‎Tests/LibWeb/Text/expected/wpt-import/html/semantics/tabular-data/the-table-element/table-rows.txt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Harness status: OK
2+
3+
Found 5 tests
4+
5+
5 Pass
6+
Pass Children of table
7+
Pass Children of thead
8+
Pass Children of tbody
9+
Pass Children of tfoot
10+
Pass Complicated case

‎Tests/LibWeb/Text/input/wpt-import/html/semantics/tabular-data/the-table-element/table-rows.html‎

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
<!DOCTYPE html>
2+
<title>HTMLTableElement.rows</title>
3+
<script src="../../../../resources/testharness.js"></script>
4+
<script src="../../../../resources/testharnessreport.js"></script>
5+
<div id="log"></div>
6+
<script>
7+
function assert_nodelist_equals(actual, expected) {
8+
assert_equals(actual.length, expected.length);
9+
10+
for (var i = 0; i < actual.length; ++i) {
11+
assert_true(i in actual);
12+
assert_true(actual.hasOwnProperty(i),
13+
"property " + i + " expected to be present on the object");
14+
assert_equals(actual.item(i), expected[i]);
15+
assert_equals(actual[i], expected[i]);
16+
}
17+
}
18+
19+
function test_table_simple(group, table) {
20+
var foo1 = group.appendChild(document.createElement("tr"));
21+
foo1.id = "foo";
22+
var bar1 = group.appendChild(document.createElement("tr"));
23+
bar1.id = "bar";
24+
var foo2 = group.appendChild(document.createElement("tr"));
25+
foo2.id = "foo";
26+
var bar2 = group.appendChild(document.createElement("tr"));
27+
bar2.id = "bar";
28+
29+
assert_true(table.rows instanceof HTMLCollection, "table.rows should be a HTMLCollection.");
30+
assert_nodelist_equals(table.rows, [foo1, bar1, foo2, bar2]);
31+
assert_equals(table.rows.foo, foo1);
32+
assert_equals(table.rows["foo"], foo1);
33+
assert_equals(table.rows.namedItem("foo"), foo1);
34+
assert_equals(table.rows.bar, bar1);
35+
assert_equals(table.rows["bar"], bar1);
36+
assert_equals(table.rows.namedItem("bar"), bar1);
37+
assert_array_equals(Object.getOwnPropertyNames(table.rows), [
38+
"0",
39+
"1",
40+
"2",
41+
"3",
42+
"foo",
43+
"bar"
44+
]);
45+
}
46+
test(function() {
47+
var table = document.createElement("table");
48+
test_table_simple(table, table);
49+
}, "Children of table");
50+
test(function() {
51+
var table = document.createElement("table");
52+
var group = table.appendChild(document.createElement("thead"));
53+
test_table_simple(group, table);
54+
}, "Children of thead");
55+
test(function() {
56+
var table = document.createElement("table");
57+
var group = table.appendChild(document.createElement("tbody"));
58+
test_table_simple(group, table);
59+
}, "Children of tbody");
60+
test(function() {
61+
var table = document.createElement("table");
62+
var group = table.appendChild(document.createElement("tfoot"));
63+
test_table_simple(group, table);
64+
}, "Children of tfoot");
65+
test(function() {
66+
var table = document.createElement("table");
67+
var orphan1 = table.appendChild(document.createElement("tr"));
68+
orphan1.id = "orphan1";
69+
var foot1 = table.appendChild(document.createElement("tfoot"));
70+
var orphan2 = table.appendChild(document.createElement("tr"));
71+
orphan2.id = "orphan2";
72+
var foot2 = table.appendChild(document.createElement("tfoot"));
73+
var orphan3 = table.appendChild(document.createElement("tr"));
74+
orphan3.id = "orphan3";
75+
var body1 = table.appendChild(document.createElement("tbody"));
76+
var orphan4 = table.appendChild(document.createElement("tr"));
77+
orphan4.id = "orphan4";
78+
var body2 = table.appendChild(document.createElement("tbody"));
79+
var orphan5 = table.appendChild(document.createElement("tr"));
80+
orphan5.id = "orphan5";
81+
var head1 = table.appendChild(document.createElement("thead"));
82+
var orphan6 = table.appendChild(document.createElement("tr"));
83+
orphan6.id = "orphan6";
84+
var head2 = table.appendChild(document.createElement("thead"));
85+
var orphan7 = table.appendChild(document.createElement("tr"));
86+
orphan7.id = "orphan7";
87+
88+
var foot1row1 = foot1.appendChild(document.createElement("tr"));
89+
foot1row1.id = "foot1row1";
90+
var foot1row2 = foot1.appendChild(document.createElement("tr"));
91+
foot1row2.id = "foot1row2";
92+
var foot2row1 = foot2.appendChild(document.createElement("tr"));
93+
foot2row1.id = "foot2row1";
94+
var foot2row2 = foot2.appendChild(document.createElement("tr"));
95+
foot2row2.id = "foot2row2";
96+
97+
var body1row1 = body1.appendChild(document.createElement("tr"));
98+
body1row1.id = "body1row1";
99+
var body1row2 = body1.appendChild(document.createElement("tr"));
100+
body1row2.id = "body1row2";
101+
var body2row1 = body2.appendChild(document.createElement("tr"));
102+
body2row1.id = "body2row1";
103+
var body2row2 = body2.appendChild(document.createElement("tr"));
104+
body2row2.id = "body2row2";
105+
106+
var head1row1 = head1.appendChild(document.createElement("tr"));
107+
head1row1.id = "head1row1";
108+
var head1row2 = head1.appendChild(document.createElement("tr"));
109+
head1row2.id = "head1row2";
110+
var head2row1 = head2.appendChild(document.createElement("tr"));
111+
head2row1.id = "head2row1";
112+
var head2row2 = head2.appendChild(document.createElement("tr"));
113+
head2row2.id = "head2row2";
114+
115+
// These elements should not end up in any collection.
116+
table.appendChild(document.createElement("div"))
117+
.appendChild(document.createElement("tr"));
118+
foot1.appendChild(document.createElement("div"))
119+
.appendChild(document.createElement("tr"));
120+
body1.appendChild(document.createElement("div"))
121+
.appendChild(document.createElement("tr"));
122+
head1.appendChild(document.createElement("div"))
123+
.appendChild(document.createElement("tr"));
124+
table.appendChild(document.createElementNS("http://example.com/test", "tr"));
125+
foot1.appendChild(document.createElementNS("http://example.com/test", "tr"));
126+
body1.appendChild(document.createElementNS("http://example.com/test", "tr"));
127+
head1.appendChild(document.createElementNS("http://example.com/test", "tr"));
128+
129+
assert_true(table.rows instanceof HTMLCollection, "table.rows should be a HTMLCollection.");
130+
assert_nodelist_equals(table.rows, [
131+
// thead
132+
head1row1,
133+
head1row2,
134+
head2row1,
135+
head2row2,
136+
137+
// tbody + table
138+
orphan1,
139+
orphan2,
140+
orphan3,
141+
body1row1,
142+
body1row2,
143+
orphan4,
144+
body2row1,
145+
body2row2,
146+
orphan5,
147+
orphan6,
148+
orphan7,
149+
150+
// tfoot
151+
foot1row1,
152+
foot1row2,
153+
foot2row1,
154+
foot2row2
155+
]);
156+
assert_array_equals(Object.getOwnPropertyNames(table.rows), [
157+
"0",
158+
"1",
159+
"2",
160+
"3",
161+
"4",
162+
"5",
163+
"6",
164+
"7",
165+
"8",
166+
"9",
167+
"10",
168+
"11",
169+
"12",
170+
"13",
171+
"14",
172+
"15",
173+
"16",
174+
"17",
175+
"18",
176+
"head1row1",
177+
"head1row2",
178+
"head2row1",
179+
"head2row2",
180+
"orphan1",
181+
"orphan2",
182+
"orphan3",
183+
"body1row1",
184+
"body1row2",
185+
"orphan4",
186+
"body2row1",
187+
"body2row2",
188+
"orphan5",
189+
"orphan6",
190+
"orphan7",
191+
"foot1row1",
192+
"foot1row2",
193+
"foot2row1",
194+
"foot2row2"
195+
]);
196+
197+
var ids = [
198+
"orphan1",
199+
"orphan2",
200+
"orphan3",
201+
"orphan4",
202+
"orphan5",
203+
"orphan6",
204+
"orphan7",
205+
"foot1row1",
206+
"foot1row2",
207+
"foot2row1",
208+
"foot2row2",
209+
"body1row1",
210+
"body1row2",
211+
"body2row1",
212+
"body2row2",
213+
"head1row1",
214+
"head1row2",
215+
"head2row1",
216+
"head2row2"
217+
];
218+
ids.forEach(function(id) {
219+
assert_equals(table.rows.namedItem(id).id, id);
220+
assert_true(id in table.rows);
221+
assert_equals(table.rows[id].id, id);
222+
assert_true(id in table.rows);
223+
});
224+
while (table.firstChild) {
225+
table.removeChild(table.firstChild);
226+
}
227+
ids.forEach(function(id) {
228+
assert_equals(table.rows.namedItem(id), null);
229+
assert_false(id in table.rows);
230+
assert_equals(table.rows[id], undefined);
231+
assert_false(id in table.rows);
232+
});
233+
}, "Complicated case");
234+
</script>

0 commit comments

Comments
 (0)