LibWeb: Detach stale layout nodes from DOM during layout tree build
When toggling `display: none` on an element, it can go from having a layout subtree to not having one. In the `none` case, we were previously leaving stale layout nodes hanging off DOM nodes in the subtree. These layout nodes could be queried for outdated information and probably other things that we shouldn't allow. Fix this by having TreeBuilder prune any old layout nodes hanging off nodes in a subtree after its subtree root doesn't produce a layout node.
This commit is contained in:
parent
0805060e5e
commit
28fdc7af05
Notes:
sideshowbarker
2024-07-17 01:12:07 +09:00
Author: https://github.com/awesomekling Commit: https://github.com/SerenityOS/serenity/commit/28fdc7af05 Pull-request: https://github.com/SerenityOS/serenity/pull/20296
5 changed files with 33 additions and 3 deletions
|
@ -0,0 +1,2 @@
|
|||
100
|
||||
0
|
|
@ -0,0 +1,15 @@
|
|||
<script src="include.js"></script>
|
||||
<script>
|
||||
test(() => {
|
||||
let a = document.createElement("div");
|
||||
let b = document.createElement("div");
|
||||
a.appendChild(b);
|
||||
document.body.appendChild(a);
|
||||
|
||||
b.style = 'width: 100px;';
|
||||
|
||||
println(b.offsetWidth);
|
||||
a.style.display = 'none';
|
||||
println(b.offsetWidth);
|
||||
});
|
||||
</script>
|
|
@ -882,7 +882,7 @@ void Node::set_layout_node(Badge<Layout::Node>, JS::NonnullGCPtr<Layout::Node> l
|
|||
m_layout_node = layout_node;
|
||||
}
|
||||
|
||||
void Node::detach_layout_node(Badge<DOM::Document>)
|
||||
void Node::detach_layout_node(Badge<Layout::TreeBuilder>)
|
||||
{
|
||||
m_layout_node = nullptr;
|
||||
}
|
||||
|
|
|
@ -185,7 +185,7 @@ public:
|
|||
Painting::Paintable const* paintable() const;
|
||||
|
||||
void set_layout_node(Badge<Layout::Node>, JS::NonnullGCPtr<Layout::Node>);
|
||||
void detach_layout_node(Badge<DOM::Document>);
|
||||
void detach_layout_node(Badge<Layout::TreeBuilder>);
|
||||
|
||||
virtual bool is_child_allowed(Node const&) const { return true; }
|
||||
|
||||
|
|
|
@ -210,8 +210,22 @@ ErrorOr<void> TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element
|
|||
|
||||
ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& context)
|
||||
{
|
||||
JS::GCPtr<Layout::Node> layout_node;
|
||||
Optional<TemporaryChange<bool>> has_svg_root_change;
|
||||
|
||||
ScopeGuard remove_stale_layout_node_guard = [&] {
|
||||
// If we didn't create a layout node for this DOM node,
|
||||
// go through the DOM tree and remove any old layout nodes since they are now all stale.
|
||||
if (!layout_node) {
|
||||
dom_node.for_each_in_inclusive_subtree([&](auto& node) {
|
||||
node.detach_layout_node({});
|
||||
if (is<DOM::Element>(node))
|
||||
static_cast<DOM::Element&>(node).clear_pseudo_element_nodes({});
|
||||
return IterationDecision::Continue;
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
if (dom_node.is_svg_container()) {
|
||||
has_svg_root_change.emplace(context.has_svg_root, true);
|
||||
} else if (dom_node.requires_svg_container() && !context.has_svg_root) {
|
||||
|
@ -220,7 +234,6 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
|
|||
|
||||
auto& document = dom_node.document();
|
||||
auto& style_computer = document.style_computer();
|
||||
JS::GCPtr<Layout::Node> layout_node;
|
||||
RefPtr<CSS::StyleProperties> style;
|
||||
CSS::Display display;
|
||||
|
||||
|
|
Loading…
Add table
Reference in a new issue