LibWeb/Layout: Replace existing ::backdrop layout nodes when necessary
We had two issues with ::backdrop which this commit fixes: ::backdrop is unique in that it's the previous sibling to its originating element, instead of a child of it. This means when that element's layout node is thrown away, the ::backdrop's is not. A second issue is that if we do a partial layout rebuild, the originating element's layout node replaces its previous one, but we would still append a new layout node for ::backdrop to the root, so it would appear in front of the originating element. A related issue is that clear_pseudo_element_nodes() got called on the element after its ::backdrop had been assigned, so it would immediately lose track of it again. To solve this, we now always remove the ::backdrop's layout node. If we need to create a new one, we insert it before the element's layout node if it has one, otherwise we append as before. This ensures we only ever have up to one layout node for the ::backdrop, and it appears behind its originating element. To support this, create_pseudo_element_if_needed() has a couple of changes: - It returns the node that was created. - The caller can ask it not to insert the node, so that the caller can do so (which we use so that we can insert it in a specific place)
This commit is contained in:
parent
b4a3520cc1
commit
b6207201d6
5 changed files with 96 additions and 20 deletions
|
|
@ -272,13 +272,13 @@ private:
|
|||
|
||||
GC_DEFINE_ALLOCATOR(GeneratedContentImageProvider);
|
||||
|
||||
void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::PseudoElement pseudo_element, AppendOrPrepend mode)
|
||||
GC::Ptr<NodeWithStyle> TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::PseudoElement pseudo_element, Optional<AppendOrPrepend> insertion_mode)
|
||||
{
|
||||
auto& document = element.document();
|
||||
|
||||
auto pseudo_element_style = element.computed_properties(pseudo_element);
|
||||
if (!pseudo_element_style)
|
||||
return;
|
||||
return {};
|
||||
|
||||
auto initial_quote_nesting_level = m_quote_nesting_level;
|
||||
DOM::AbstractElement element_reference { element, pseudo_element };
|
||||
|
|
@ -293,12 +293,12 @@ void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Ps
|
|||
&& (pseudo_element_display.is_none()
|
||||
|| pseudo_element_content.type == CSS::ContentData::Type::Normal
|
||||
|| pseudo_element_content.type == CSS::ContentData::Type::None))
|
||||
return;
|
||||
return {};
|
||||
|
||||
// For ::marker with content or display 'none' -- do nothing.
|
||||
if (pseudo_element == CSS::PseudoElement::Marker
|
||||
&& (pseudo_element_display.is_none() || pseudo_element_content.type == CSS::ContentData::Type::None))
|
||||
return;
|
||||
return {};
|
||||
|
||||
// For ::marker with content 'normal', create the marker pseudo-element from a ListItemMarkerBox
|
||||
// FIXME: This + ListItemBox + ListItemMarkerBox will disappear once ::marker pseudo-elements with 'normal' content
|
||||
|
|
@ -317,12 +317,12 @@ void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Ps
|
|||
element.set_computed_properties(CSS::PseudoElement::Marker, pseudo_element_style);
|
||||
element.set_pseudo_element_node({}, CSS::PseudoElement::Marker, list_item_marker);
|
||||
list_box->prepend_child(*list_item_marker);
|
||||
return;
|
||||
return list_item_marker;
|
||||
}
|
||||
|
||||
auto pseudo_element_node = DOM::Element::create_layout_node_for_display_type(document, pseudo_element_display, *pseudo_element_style, nullptr);
|
||||
if (!pseudo_element_node)
|
||||
return;
|
||||
return {};
|
||||
|
||||
// FIXME: This code actually computes style for element::marker, and shouldn't for element::pseudo::marker
|
||||
if (is<ListItemBox>(*pseudo_element_node)) {
|
||||
|
|
@ -346,7 +346,8 @@ void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Ps
|
|||
pseudo_element_node->set_initial_quote_nesting_level(initial_quote_nesting_level);
|
||||
|
||||
element.set_pseudo_element_node({}, pseudo_element, pseudo_element_node);
|
||||
insert_node_into_inline_or_block_ancestor(*pseudo_element_node, pseudo_element_display, mode);
|
||||
if (insertion_mode.has_value())
|
||||
insert_node_into_inline_or_block_ancestor(*pseudo_element_node, pseudo_element_display, insertion_mode.value());
|
||||
pseudo_element_node->mutable_computed_values().set_content(pseudo_element_content);
|
||||
|
||||
CSS::resolve_counters(element_reference);
|
||||
|
|
@ -378,6 +379,8 @@ void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Ps
|
|||
TODO();
|
||||
}
|
||||
}
|
||||
|
||||
return pseudo_element_node;
|
||||
}
|
||||
|
||||
// Block nodes inside inline nodes are allowed, but to maintain the invariant that either all layout children are
|
||||
|
|
@ -621,6 +624,10 @@ void TreeBuilder::update_layout_tree(DOM::Node& dom_node, TreeBuilder::Context&
|
|||
} else {
|
||||
if (is<DOM::Element>(dom_node)) {
|
||||
auto& element = static_cast<DOM::Element&>(dom_node);
|
||||
// ::backdrop is a sibling of the element, not a child, so unlike other pseudo-elements, it's not
|
||||
// automatically discarded when element's layout is recomputed. We must remove it manually.
|
||||
if (auto old_backdrop_node = element.get_pseudo_element_node(CSS::PseudoElement::Backdrop))
|
||||
old_backdrop_node->remove();
|
||||
element.clear_pseudo_element_nodes({});
|
||||
VERIFY(!element.needs_style_update());
|
||||
style = element.computed_properties();
|
||||
|
|
@ -653,14 +660,31 @@ void TreeBuilder::update_layout_tree(DOM::Node& dom_node, TreeBuilder::Context&
|
|||
if (!layout_node)
|
||||
return;
|
||||
|
||||
// Decide whether to replace an existing node (partial tree update) or insert a new one appropriately.
|
||||
bool const may_replace_existing_layout_node = must_create_subtree == MustCreateSubtree::No
|
||||
&& old_layout_node
|
||||
&& old_layout_node->parent()
|
||||
&& old_layout_node != layout_node;
|
||||
|
||||
if (dom_node.is_element() && should_create_layout_node) {
|
||||
auto& element = static_cast<DOM::Element&>(dom_node);
|
||||
// Each element rendered in the top layer has a ::backdrop pseudo-element, for which it is the originating element.
|
||||
if (element.rendered_in_top_layer() && context.layout_top_layer) {
|
||||
// If we're inserting a new element, we can append the ::backdrop node now, before layout_node is appended.
|
||||
// Otherwise, we need to insert the ::backdrop before old_layout_node so it's behind the layout_node.
|
||||
if (may_replace_existing_layout_node) {
|
||||
if (auto backdrop_node = create_pseudo_element_if_needed(element, CSS::PseudoElement::Backdrop, {})) {
|
||||
old_layout_node->parent()->insert_before(*backdrop_node, old_layout_node);
|
||||
}
|
||||
} else {
|
||||
create_pseudo_element_if_needed(element, CSS::PseudoElement::Backdrop, AppendOrPrepend::Append);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (dom_node.is_document()) {
|
||||
m_layout_root = layout_node;
|
||||
} else if (should_create_layout_node) {
|
||||
// Decide whether to replace an existing node (partial tree update) or insert a new one appropriately.
|
||||
bool const may_replace_existing_layout_node = must_create_subtree == MustCreateSubtree::No
|
||||
&& old_layout_node
|
||||
&& old_layout_node->parent()
|
||||
&& old_layout_node != layout_node;
|
||||
if (may_replace_existing_layout_node) {
|
||||
old_layout_node->parent()->replace_child(*layout_node, *old_layout_node);
|
||||
} else if (layout_node->is_svg_box()) {
|
||||
|
|
@ -712,14 +736,8 @@ void TreeBuilder::update_layout_tree(DOM::Node& dom_node, TreeBuilder::Context&
|
|||
// generate boxes as if they were siblings of the root element.
|
||||
TemporaryChange<bool> layout_mask(context.layout_top_layer, true);
|
||||
for (auto const& top_layer_element : document.top_layer_elements()) {
|
||||
if (top_layer_element->rendered_in_top_layer()) {
|
||||
// Each element rendered in the top layer has a ::backdrop pseudo-element, for which it is the originating element.
|
||||
if ((should_create_layout_node || top_layer_element->needs_layout_tree_update())
|
||||
&& !top_layer_element->has_inclusive_ancestor_with_display_none()) {
|
||||
create_pseudo_element_if_needed(top_layer_element, CSS::PseudoElement::Backdrop, AppendOrPrepend::Append);
|
||||
}
|
||||
if (top_layer_element->rendered_in_top_layer())
|
||||
update_layout_tree(top_layer_element, context, should_create_layout_node ? MustCreateSubtree::Yes : MustCreateSubtree::No);
|
||||
}
|
||||
}
|
||||
}
|
||||
pop_parent();
|
||||
|
|
|
|||
|
|
@ -58,7 +58,7 @@ private:
|
|||
Prepend,
|
||||
};
|
||||
void insert_node_into_inline_or_block_ancestor(Layout::Node&, CSS::Display, AppendOrPrepend);
|
||||
void create_pseudo_element_if_needed(DOM::Element&, CSS::PseudoElement, AppendOrPrepend);
|
||||
GC::Ptr<NodeWithStyle> create_pseudo_element_if_needed(DOM::Element&, CSS::PseudoElement, Optional<AppendOrPrepend>);
|
||||
void restructure_block_node_in_inline_parent(NodeWithStyleAndBoxModelMetrics&);
|
||||
|
||||
GC::Ptr<Layout::Node> m_layout_root;
|
||||
|
|
|
|||
18
Tests/LibWeb/Crash/CSS/backdrop-display-none.html
Normal file
18
Tests/LibWeb/Crash/CSS/backdrop-display-none.html
Normal file
|
|
@ -0,0 +1,18 @@
|
|||
<!DOCTYPE html>
|
||||
<dialog id="d"><div id="x"></div></dialog>
|
||||
<script>
|
||||
d.showModal();
|
||||
document.body.offsetWidth;
|
||||
|
||||
const style = document.createElement('style');
|
||||
style.textContent = "dialog::backdrop { display: none }";
|
||||
document.head.appendChild(style);
|
||||
document.body.offsetWidth;
|
||||
|
||||
x.hidden = true;
|
||||
document.body.offsetWidth;
|
||||
|
||||
// And now show it again, to ensure that works
|
||||
style.textContent = "";
|
||||
document.body.offsetWidth;
|
||||
</script>
|
||||
|
|
@ -0,0 +1,27 @@
|
|||
Viewport <#document> at [0,0] [0+0+0 800 0+0+0] [0+0+0 600 0+0+0] [BFC] children: not-inline
|
||||
BlockContainer <html> at [0,0] [0+0+0 800 0+0+0] [0+0+0 16 0+0+0] [BFC] children: not-inline
|
||||
BlockContainer <body> at [8,8] [8+0+0 784 0+0+8] [8+0+0 0 0+0+8] children: inline
|
||||
TextNode <#text> (not painted)
|
||||
BlockContainer <(anonymous)> at [0,0] positioned [0+0+0 800 0+0+0] [0+0+0 600 0+0+0] [BFC] children: not-inline
|
||||
BlockContainer <dialog#d> at [299,289] positioned [280+3+16 202 16+3+280] [270+3+16 22 16+3+270] [BFC] children: inline
|
||||
frag 0 from BlockContainer start: 0, length: 0, rect: [300,290 200x20] baseline: 14.796875
|
||||
BlockContainer <input> at [300,290] inline-block [0+1+0 200 0+1+0] [0+1+0 20 0+1+0] [BFC] children: not-inline
|
||||
Box <div> at [302,291] flex-container(row) [0+0+2 196 2+0+0] [0+0+1 18 1+0+0] [FFC] children: not-inline
|
||||
BlockContainer <div> at [302,291] flex-item [0+0+0 196 0+0+0] [0+0+0 18 0+0+0] [BFC] children: inline
|
||||
frag 0 from TextNode start: 0, length: 0, rect: [302,291 0x18] baseline: 13.796875
|
||||
TextNode <#text> (not painted)
|
||||
|
||||
ViewportPaintable (Viewport<#document>) [0,0 800x600]
|
||||
PaintableWithLines (BlockContainer<HTML>) [0,0 800x16]
|
||||
PaintableWithLines (BlockContainer<BODY>) [8,8 784x0]
|
||||
PaintableWithLines (BlockContainer(anonymous)) [0,0 800x600]
|
||||
PaintableWithLines (BlockContainer<DIALOG>#d) [280,270 240x60]
|
||||
PaintableWithLines (BlockContainer<INPUT>) [299,289 202x22]
|
||||
PaintableBox (Box<DIV>) [300,290 200x20]
|
||||
PaintableWithLines (BlockContainer<DIV>) [302,291 196x18]
|
||||
TextPaintable (TextNode<#text>)
|
||||
|
||||
SC for Viewport<#document> [0,0 800x600] [children: 3] (z-index: auto)
|
||||
SC for BlockContainer<HTML> [0,0 800x16] [children: 0] (z-index: auto)
|
||||
SC for BlockContainer(anonymous) [0,0 800x600] [children: 0] (z-index: auto)
|
||||
SC for BlockContainer<DIALOG>#d [299,289 202x22] [children: 0] (z-index: auto)
|
||||
|
|
@ -0,0 +1,13 @@
|
|||
<!DOCTYPE html>
|
||||
<style>
|
||||
dialog::backdrop {
|
||||
background: rgba(0,0,0,0.5)
|
||||
}
|
||||
</style><dialog id="d"><input><div id="x"></div></dialog><script>
|
||||
d.showModal();
|
||||
|
||||
for (let i = 0; i < 5; ++i) {
|
||||
x.hidden = !x.hidden;
|
||||
document.body.offsetWidth; // force layout
|
||||
}
|
||||
</script>
|
||||
Loading…
Reference in a new issue