LibWeb: Move use pseudo element styles from TreeBuilder to StyleComputer

The styling of elements using the `use_pseudo_element()` was only
applied on layout. When an element style was recomputed later that
styling was not overruled with the pseudo element selector styles.
This moves the styling override from `TreeBuilder.cpp` to
`StyleComputer.cpp`. Now the styles are always correctly applied.
I also removed the method `property_id_by_index()` because it was
not needed anymore.

Als some calls to `invalidate_layout()` in the Meter, Progress and
Select elements where not needed anymore because the style values
are update on the changing of the style attribute.

This fixes issue #22278.
This commit is contained in:
Bastiaan van der Plaat 2023-12-17 21:03:28 +01:00 committed by Andreas Kling
parent 7578620f25
commit a05fd28b7b
Notes: sideshowbarker 2024-07-17 05:05:51 +09:00
11 changed files with 44 additions and 49 deletions

View file

@ -0,0 +1,17 @@
Viewport <#document> at (0,0) content-size 800x600 children: not-inline
BlockContainer <html> at (0,0) content-size 800x600 [BFC] children: not-inline
BlockContainer <body> at (8,8) content-size 784x21.84375 children: inline
line 0 width: 300, height: 21.84375, bottom: 21.84375, baseline: 16.921875
frag 0 from BlockContainer start: 0, length: 0, rect: [8,12 300x12]
BlockContainer <progress#a> at (8,12) content-size 300x12 inline-block [BFC] children: not-inline
BlockContainer <div> at (9,13) content-size 298x12 children: not-inline
BlockContainer <div> at (9,13) content-size 298x12 children: not-inline
TextNode <#text>
TextNode <#text>
ViewportPaintable (Viewport<#document>) [0,0 800x600]
PaintableWithLines (BlockContainer<HTML>) [0,0 800x600]
PaintableWithLines (BlockContainer<BODY>) [8,8 784x21.84375]
PaintableWithLines (BlockContainer<PROGRESS>#a) [8,12 300x12] overflow: [8,12 300x14]
PaintableWithLines (BlockContainer<DIV>) [8,12 300x14]
PaintableWithLines (BlockContainer<DIV>) [9,13 298x12]

View file

@ -0,0 +1,9 @@
<style>
* {
font: 20px 'SerenitySans';
}
</style>
<progress id="a"></progress>
<script>
a.style.backgroundColor = 'red'; // Trigger style invalidation
</script>

View file

@ -59,11 +59,6 @@ String PropertyOwningCSSStyleDeclaration::item(size_t index) const
return MUST(String::from_utf8(CSS::string_from_property_id(m_properties[index].property_id)));
}
CSS::PropertyID PropertyOwningCSSStyleDeclaration::property_id_by_index(size_t index) const
{
return m_properties[index].property_id;
}
JS::NonnullGCPtr<ElementInlineCSSStyleDeclaration> ElementInlineCSSStyleDeclaration::create(DOM::Element& element, Vector<StyleProperty> properties, HashMap<FlyString, StyleProperty> custom_properties)
{
auto& realm = element.realm();

View file

@ -24,7 +24,6 @@ public:
virtual size_t length() const = 0;
virtual String item(size_t index) const = 0;
virtual CSS::PropertyID property_id_by_index(size_t index) const = 0;
virtual Optional<StyleProperty> property(PropertyID) const = 0;
@ -64,7 +63,6 @@ public:
virtual size_t length() const override;
virtual String item(size_t index) const override;
virtual CSS::PropertyID property_id_by_index(size_t index) const override;
virtual Optional<StyleProperty> property(PropertyID) const override;

View file

@ -82,11 +82,6 @@ String ResolvedCSSStyleDeclaration::item(size_t index) const
return MUST(String::from_utf8(string_from_property_id(property_id)));
}
CSS::PropertyID ResolvedCSSStyleDeclaration::property_id_by_index(size_t index) const
{
return static_cast<PropertyID>(index + to_underlying(first_longhand_property_id));
}
static NonnullRefPtr<StyleValue const> style_value_for_background_property(Layout::NodeWithStyle const& layout_node, Function<NonnullRefPtr<StyleValue const>(BackgroundLayerData const&)> callback, Function<NonnullRefPtr<StyleValue const>()> default_value)
{
auto const& background_layers = layout_node.background_layers();

View file

@ -21,7 +21,6 @@ public:
virtual size_t length() const override;
virtual String item(size_t index) const override;
virtual CSS::PropertyID property_id_by_index(size_t index) const override;
virtual Optional<StyleProperty> property(PropertyID) const override;
virtual WebIDL::ExceptionOr<void> set_property(PropertyID, StringView css_text, StringView priority) override;

View file

@ -2136,6 +2136,20 @@ ErrorOr<RefPtr<StyleProperties>> StyleComputer::compute_style_impl(DOM::Element&
{
build_rule_cache_if_needed();
// Special path for elements that use pseudo element as style selector
if (element.use_pseudo_element().has_value()) {
auto& parent_element = verify_cast<HTML::HTMLElement>(*element.root().parent_or_shadow_host());
auto style = TRY(compute_style(parent_element, *element.use_pseudo_element()));
// Merge back inline styles
if (element.has_attribute(HTML::AttributeNames::style)) {
auto* inline_style = parse_css_style_attribute(CSS::Parser::ParsingContext(document()), *element.get_attribute(HTML::AttributeNames::style), element);
for (auto const& property : inline_style->properties())
style->set_property(property.property_id, property.value);
}
return style;
}
auto style = StyleProperties::create();
// 1. Perform the cascade. This produces the "specified style"
bool did_match_any_pseudo_element_rules = false;

View file

@ -55,7 +55,6 @@ WebIDL::ExceptionOr<void> HTMLMeterElement::set_value(double value)
{
TRY(set_attribute(HTML::AttributeNames::value, MUST(String::number(value))));
update_meter_value_element();
document().invalidate_layout();
return {};
}
@ -74,7 +73,6 @@ WebIDL::ExceptionOr<void> HTMLMeterElement::set_min(double value)
{
TRY(set_attribute(HTML::AttributeNames::min, MUST(String::number(value))));
update_meter_value_element();
document().invalidate_layout();
return {};
}
@ -96,7 +94,6 @@ WebIDL::ExceptionOr<void> HTMLMeterElement::set_max(double value)
{
TRY(set_attribute(HTML::AttributeNames::max, MUST(String::number(value))));
update_meter_value_element();
document().invalidate_layout();
return {};
}
@ -120,7 +117,6 @@ WebIDL::ExceptionOr<void> HTMLMeterElement::set_low(double value)
{
TRY(set_attribute(HTML::AttributeNames::low, MUST(String::number(value))));
update_meter_value_element();
document().invalidate_layout();
return {};
}
@ -144,7 +140,6 @@ WebIDL::ExceptionOr<void> HTMLMeterElement::set_high(double value)
{
TRY(set_attribute(HTML::AttributeNames::high, MUST(String::number(value))));
update_meter_value_element();
document().invalidate_layout();
return {};
}
@ -168,7 +163,6 @@ WebIDL::ExceptionOr<void> HTMLMeterElement::set_optimum(double value)
{
TRY(set_attribute(HTML::AttributeNames::optimum, MUST(String::number(value))));
update_meter_value_element();
document().invalidate_layout();
return {};
}

View file

@ -53,7 +53,6 @@ WebIDL::ExceptionOr<void> HTMLProgressElement::set_value(double value)
TRY(set_attribute(HTML::AttributeNames::value, MUST(String::number(value))));
update_progress_value_element();
document().invalidate_layout();
return {};
}
@ -74,7 +73,6 @@ WebIDL::ExceptionOr<void> HTMLProgressElement::set_max(double value)
TRY(set_attribute(HTML::AttributeNames::max, MUST(String::number(value))));
update_progress_value_element();
document().invalidate_layout();
return {};
}

View file

@ -213,7 +213,6 @@ WebIDL::ExceptionOr<void> HTMLSelectElement::set_value(String const& value)
for (auto const& option_element : list_of_options())
option_element->set_selected(option_element->value() == value);
update_inner_text_element();
document().invalidate_layout();
// When the user agent is to send select update notifications, queue an element task on the user interaction task source given the select element to run these steps:
queue_an_element_task(HTML::Task::Source::UserInteraction, [this] {
@ -323,7 +322,6 @@ void HTMLSelectElement::form_associated_element_was_inserted()
if (options.size() > 0) {
options.at(0)->set_selected(true);
update_inner_text_element();
document().invalidate_layout();
}
}
});

View file

@ -313,32 +313,10 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
if (is<DOM::Element>(dom_node)) {
auto& element = static_cast<DOM::Element&>(dom_node);
// Special path for elements that use pseudo element as style selector.
if (element.use_pseudo_element().has_value()) {
// Get base psuedo element selector style properties
auto& parent_element = verify_cast<HTML::HTMLElement>(*element.root().parent_or_shadow_host());
style = TRY(style_computer.compute_style(parent_element, *element.use_pseudo_element()));
// Merge back inline styles
auto const* inline_style = element.inline_style();
if (inline_style) {
auto const& computed_style = element.computed_css_values();
for (size_t i = 0; i < inline_style->length(); i++) {
auto property_id = inline_style->property_id_by_index(i);
if (auto property = computed_style->maybe_null_property(property_id); property)
style->set_property(property_id, *property);
}
}
display = style->display();
}
// Common path: this is a regular DOM element. Style should be present already, thanks to Document::update_style().
else {
element.clear_pseudo_element_nodes({});
VERIFY(!element.needs_style_update());
style = element.computed_css_values();
display = style->display();
}
element.clear_pseudo_element_nodes({});
VERIFY(!element.needs_style_update());
style = element.computed_css_values();
display = style->display();
if (display.is_none())
return {};
layout_node = element.create_layout_node(*style);