Skip to content

GitLab

T
tangled
Commit 504597ba authored by Yu Han's avatar Yu Han Committed by Commit Bot
Browse files
Options

Adds validity check to imperative slot API. Throws exception when failed. 

Prior to this CL, calling imperative slot API does not check the
validity of the slot element or its assigned nodes. However, after
the last TPAC Meeting,
https://github.com/whatwg/html/issues/3534#issuecomment-537802687,
the members agree to not allow manual assignment of any light-tree
descendant other than the host's direct light-tree children.

This CL validates the slot element and its assigned nodes when calling
imperative slot API. The checks are: shadow root is V1, shadow root is
in manual slot assignment mode, and assign nodes are children of slot's
host element.

Bug: 869308
Change-Id: I4b0a1a1a3af6bf159e22af5eafe32ed44172b021
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2096958Reviewed-by: default avatarHayato Ito <hayato@chromium.org>
Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Commit-Queue: Yu Han <yuzhehan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750637}
parent 42287716
Hide whitespace changes
Inline Side-by-side
Showing with 46 additions and 62 deletions
third_party/blink/renderer/core/html/html_slot_element.cc
View file @ 504597ba
......@@ -171,13 +171,33 @@ const HeapVector<Member<Element>> HTMLSlotElement::AssignedElementsForBinding(
return elements;
}
void HTMLSlotElement::assign(HeapVector<Member<Node>> nodes) {
if (SupportsAssignment())
ContainingShadowRoot()->GetSlotAssignment().SetNeedsAssignmentRecalc();
void HTMLSlotElement::assign(HeapVector<Member<Node>> nodes,
ExceptionState& exception_state) {
if (!SupportsAssignment() || !ContainingShadowRoot()->IsManualSlotting()) {
exception_state.ThrowDOMException(
DOMExceptionCode::kNotAllowedError,
"This shadow root does not support manual slot assignment.");
return;
}
assigned_nodes_candidates_.clear();
auto* host = OwnerShadowHost();
bool has_invalid_node = false;
for (auto& node : nodes) {
if (node->parentNode() != host) {
exception_state.ThrowDOMException(
DOMExceptionCode::kNotAllowedError,
"Node: '" + node->nodeName() +
"' is invalid for manual slot assignment.");
assigned_nodes_candidates_.clear();
has_invalid_node = true;
break;
}
assigned_nodes_candidates_.insert(node);
}
if (!has_invalid_node)
ContainingShadowRoot()->GetSlotAssignment().SetNeedsAssignmentRecalc();
}
void HTMLSlotElement::AppendAssignedNode(Node& host_child) {
......
third_party/blink/renderer/core/html/html_slot_element.h
View file @ 504597ba
......@@ -110,7 +110,7 @@ class CORE_EXPORT HTMLSlotElement final : public HTMLElement {
static const AtomicString& UserAgentDefaultSlotName();
// For imperative Shadow DOM distribution APIs
void assign(HeapVector<Member<Node>> nodes);
void assign(HeapVector<Member<Node>> nodes, ExceptionState&);
const HeapHashSet<Member<Node>>& AssignedNodesCandidate() const {
return assigned_nodes_candidates_;
}
......
third_party/blink/renderer/core/html/html_slot_element.idl
View file @ 504597ba
......@@ -30,5 +30,5 @@ interface HTMLSlotElement : HTMLElement {
[CEReactions, Reflect] attribute DOMString name;
[ImplementedAs=AssignedNodesForBinding] sequence<Node> assignedNodes(optional AssignedNodesOptions options = {});
[ImplementedAs=AssignedElementsForBinding] sequence<Element> assignedElements(optional AssignedNodesOptions options = {});
[RuntimeEnabled=ManualSlotting] void assign(sequence<Node> nodes);
[RuntimeEnabled=ManualSlotting, RaisesException] void assign(sequence<Node> nodes);
};
third_party/blink/web_tests/TestExpectations
View file @ 504597ba
......@@ -6549,10 +6549,6 @@ crbug.com/944449 virtual/threaded/external/wpt/scroll-animations/animation-with-
# Sheriff 2020-02-10
crbug.com/1050306 [ Fuchsia ] virtual/gpu/fast/canvas/canvas-filter-frameless-document.html [ Crash Pass Timeout ]
# Imperative Shadow DOM Slot API - WIP
crbug.com/869308 external/wpt/shadow-dom/slots-imperative-slot-api.tentative.html [ Failure ]
crbug.com/869308 virtual/web-components-v0-disabled/external/wpt/shadow-dom/slots-imperative-slot-api.tentative.html [ Failure ]
# Swiftshader issue.
crbug.com/1048149 external/wpt/html/browsers/the-window-object/apis-for-creating-and-navigating-browsing-contexts-by-name/open-features-non-integer-innerwidth.html [ Crash Timeout ]
crbug.com/1048149 external/wpt/html/browsers/the-window-object/apis-for-creating-and-navigating-browsing-contexts-by-name/open-features-non-integer-screeny.html [ Crash Timeout ]
......
third_party/blink/web_tests/external/wpt/shadow-dom/slots-imperative-slot-api.tentative-expected.txt
View file @ 504597ba
This is a testharness.js-based test.
PASS attachShadow can take slotAssignment parameter.
FAIL Imperative slot API throws exception when not slotAssignment != 'manual'. assert_throws_dom: function "() => { tTree.s1.assign([]); }" did not throw
FAIL Imperative slot API throws exception when slotable parentNode != slot's host. assert_throws_dom: function "() => { tTree.s2.assign([tTree.c1]); }" did not throw
FAIL Imperative slot API can assign nodes in manual slot assignment. assert_equals: expected null but got Element node <slot id="s1"></slot>
FAIL Order of slotables is preserved in manual slot assignment. assert_array_equals: lengths differ, expected array [Element node <div id="c2"></div>, Element node <div id="c3"></div>, Element node <div id="c1"></div>] length 3, got [Text node "
FAIL Previously assigned slotable is moved to new slot when it's reassigned. assert_array_equals: lengths differ, expected array [Element node <div id="c2"></div>, Element node <div id="c3"></div>, Element node <div id="c1"></div>] length 3, got [Text node "
FAIL Assigning invalid nodes causes exception and slot returns to its previous state. assert_equals: expected "NotAllowedError" but got "Error"
FAIL Moving a slot to a new host, the slot looses its previously assigned slotables. assert_array_equals: lengths differ, expected array [Element node <div id="c1"></div>, Element node <div id="c2"></div>, Element node <div id="c3"></div>] length 3, got [Text node "
FAIL Moving a slot's tree order position within a shadow host has no impact on its assigned slotables. assert_array_equals: lengths differ, expected array [Element node <div id="c1"></div>, Element node <div id="c2"></div>, Element node <div id="c3"></div>] length 3, got [Text node "
FAIL Appending slotable to different host, it looses slot assignment. It can be re-assigned within a new host. assert_array_equals: lengths differ, expected array [Element node <div id="c1"></div>, Element node <div id="c2"></div>, Element node <div id="c3"></div>] length 3, got [Text node "
FAIL Assignment with the same node in parameters should be ignored, last one wins. assert_array_equals: lengths differ, expected array [Element node <div id="c1"></div>] length 1, got [Text node "
FAIL Removing a slot from DOM resets its slotable's slot assignment. assert_equals: expected null but got Element node <slot id="s2"></slot>
PASS Imperative slot API throws exception when not slotAssignment != 'manual'.
PASS Imperative slot API throws exception when slotable parentNode != slot's host.
PASS Imperative slot API can assign nodes in manual slot assignment.
FAIL Order of slotables is preserved in manual slot assignment. assert_array_equals: expected property 0 to be Element node <div id="c2"></div> but got Element node <div id="c1"></div> (expected array [Element node <div id="c2"></div>, Element node <div id="c3"></div>, Element node <div id="c1"></div>] got [Element node <div id="c1"></div>, Element node <div id="c2"></div>, Element node <div id="c3"></div>])
FAIL Previously assigned slotable is moved to new slot when it's reassigned. assert_array_equals: expected property 0 to be Element node <div id="c2"></div> but got Element node <div id="c1"></div> (expected array [Element node <div id="c2"></div>, Element node <div id="c3"></div>, Element node <div id="c1"></div>] got [Element node <div id="c1"></div>, Element node <div id="c2"></div>, Element node <div id="c3"></div>])
FAIL Assigning invalid nodes causes exception and slot returns to its previous state. assert_array_equals: expected property 0 to be Element node <div id="c2"></div> but got Element node <div id="c1"></div> (expected array [Element node <div id="c2"></div>, Element node <div id="c3"></div>, Element node <div id="c1"></div>] got [Element node <div id="c1"></div>, Element node <div id="c2"></div>, Element node <div id="c3"></div>])
PASS Moving a slot to a new host, the slot loses its previously assigned slotables.
FAIL Moving a slot's tree order position within a shadow host has no impact on its assigned slotables. assert_array_equals: lengths differ, expected array [Element node <div id="c1"></div>, Element node <div id="c2"></div>, Element node <div id="c3"></div>] length 3, got [] length 0
FAIL Appending slotable to different host, it loses slot assignment. It can be re-assigned within a new host. Failed to execute 'assign' on 'HTMLSlotElement': The object must have a callable @@iterator property.
FAIL Assignment with the same node in parameters should be ignored, last one wins. assert_array_equals: expected property 0 to be Element node <div id="c2"></div> but got Element node <div id="c1"></div> (expected array [Element node <div id="c2"></div>, Element node <div id="c1"></div>] got [Element node <div id="c1"></div>, Element node <div id="c2"></div>])
PASS Removing a slot from DOM resets its slotable's slot assignment.
Harness: the test ran to completion.
third_party/blink/web_tests/external/wpt/shadow-dom/slots-imperative-slot-api.tentative.html
View file @ 504597ba
......@@ -28,23 +28,29 @@ test(() => {
<template data-mode="open" data-slot-assignment="auto">
<slot id="s1"></slot>
</template>
<div id="c1"></div>
</div>
<div id="host2">
<template data-mode="open" data-slot-assignment="manual">
<slot id="s2"></slot>
</template>
</div>
<div id="c1" slot="slot1"></div>
<div id="c2"></div>
</div>
<script>
test(() => {
let tTree = createTestTree(test_errors);
assert_array_equals(tTree.s1.assignedElements(), [tTree.c1]);
assert_equals(tTree.c1.assignedSlot, tTree.s1);
assert_throws_dom('NotAllowedError', () => { tTree.s1.assign([]); });
assert_array_equals(tTree.s1.assignedElements(), [tTree.c1]);
assert_equals(tTree.c1.assignedSlot, tTree.s1);
}, 'Imperative slot API throws exception when not slotAssignment != \'manual\'.');
test(() => {
let tTree = createTestTree(test_errors);
assert_throws_dom('NotAllowedError', () => { tTree.s2.assign([tTree.c1]); });
assert_throws_dom('NotAllowedError', () => { tTree.s2.assign([tTree.c2]); });
assert_throws_dom('NotAllowedError', () => { tTree.s2.assign([tTree.host1]); });
}, 'Imperative slot API throws exception when slotable parentNode != slot\'s host.');
......@@ -52,7 +58,7 @@ test(() => {
<div id="test_assign">
<div id="host">
<template id="shadow_root" data-mode="open" data--slot-assignment="manual">
<template id="shadow_root" data-mode="open" data-slot-assignment="manual">
<slot id="s1"></slot>
<slot id="s2"></slot>
<slot id="s3"></slot>
......
third_party/blink/web_tests/shadow-dom/imperative-api.html
View file @ 504597ba
......@@ -48,24 +48,6 @@ test(() => {
assert_array_equals(n.s2.assignedNodes(), [n.c1]);
}, 'assignedNodes/Slot can be used in manual slotting');
test(() => {
let n = createTestTree(test2);
n.s1.assign([n.c4]);
assert_array_equals(n.s1.assignedNodes(), []);
n.host.appendChild(n.c4);
assert_array_equals(n.s1.assignedNodes(), [n.c4]);
}, 'Nodes should be assigned to slot only when nodes are host\'s children');
test(() => {
let n = createTestTree(test2);
n.s5.assign([n.c1]);
assert_array_equals(n.s5.assignedNodes(), []);
n.shadow_root.insertBefore(n.s5,n.s1);
assert_array_equals(n.s5.assignedNodes(), [n.c1]);
}, 'Nodes should be assigned to a slot only when the slot is in host\'s shadowtree');
test(() => {
let n = createTestTree(test2);
n.s1.assign([n.c1]);
......@@ -126,24 +108,4 @@ test(() => {
assert_equals(n.c3.assignedSlot, null);
}, 'Same assignment should be ignored');
</script>
<div id="test3">
<div id="host1">
<template id="shadow_root" data-mode="open">
<slot id="s1" name="s1"></slot>
</template>
<div id="c1" slot="s1"></div>
</div>
</div>
<script>
test(() => {
let n = createTestTree(test3);
assert_array_equals(n.s1.assignedElements(), [n.c1]);
assert_equals(n.c1.assignedSlot, n.s1);
n.s1.assign([]);
assert_array_equals(n.s1.assignedElements(), [n.c1]);
assert_equals(n.c1.assignedSlot, n.s1);
}, 'Slotting API should not have any effect in auto mode');
</script>
</script>
\ No newline at end of file
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment