Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LibWeb: More animation fixes #23486

Merged
merged 6 commits into from Mar 6, 2024
Merged

LibWeb: More animation fixes #23486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ca8339d
LibWeb: Prevent transform interpolations from failing
mattco98 Mar 6, 2024
cdac0a9
AK: Add HashMap::ensure_capacity
mattco98 Mar 6, 2024
2cda21f
LibWeb: Remove unnecessary ErrorOr<> from StyleComputer
mattco98 Mar 6, 2024
5946fa6
LibWeb: Do not assert the type of transformation style values
mattco98 Mar 6, 2024
328db54
LibWeb: Consider fill state before calling Animation::play()
mattco98 Mar 6, 2024
68f1d2c
LibWeb: Don't render elements with invalid interpolated transform values
mattco98 Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions AK/HashMap.h
View file
Open in desktop
Expand Up @@ -135,6 +135,8 @@ class HashMap {

ErrorOr<void> try_ensure_capacity(size_t capacity) { return m_table.try_ensure_capacity(capacity); }

void ensure_capacity(size_t capacity) { return m_table.ensure_capacity(capacity); }

Optional<typename ValueTraits::ConstPeekType> get(K const& key) const
requires(!IsPointer<typename ValueTraits::PeekType>)
{
Expand Down
21 changes: 21 additions & 0 deletions Tests/LibWeb/Ref/css-keyframe-fill-forwards.html
View file
Open in desktop
@@ -0,0 +1,21 @@
<!doctype html>
<link rel="match" href="reference/css-keyframe-fill-forwards-ref.html" />
<style>
#foo {
width: 100px;
height: 100px;
animation: anim 1s linear forwards;
}
@keyframes anim {
from {
background-color: red;
}
to {
background-color: blue;
}
}
</style>
<div id="foo"></div>
<script>
document.getElementById("foo").getAnimations()[0].currentTime = 1500;
</script>
22 changes: 22 additions & 0 deletions Tests/LibWeb/Ref/css-keyframe-invalid-transform-should-not-render.html
View file
Open in desktop
@@ -0,0 +1,22 @@
<!doctype html>
<link rel="match" href="reference/css-keyframe-invalid-transform-should-not-render-ref.html" />
<style>
#foo {
width: 100px;
height: 100px;
background-color: blue;
animation: anim 1s;
}
@keyframes anim {
from {
transform: scale(0);
}
to {
transform: scale(0);
}
}
</style>
<div id="foo"></div>
<script>
document.getElementById("foo").getAnimations()[0].currentTime = 500;
</script>
9 changes: 9 additions & 0 deletions Tests/LibWeb/Ref/reference/css-keyframe-fill-forwards-ref.html
View file
Open in desktop
@@ -0,0 +1,9 @@
<!doctype html>
<style>
#foo {
width: 100px;
height: 100px;
background-color: blue;
}
</style>
<div id="foo"></div>
1 change: 1 addition & 0 deletions Tests/LibWeb/Ref/reference/css-keyframe-invalid-transform-should-not-render-ref.html
View file
Open in desktop
@@ -0,0 +1 @@
<!doctype html>
1 change: 1 addition & 0 deletions Tests/LibWeb/Text/expected/invalid-transform-interpolation-does-not-crash.txt
View file
Open in desktop
@@ -0,0 +1 @@
no crash!
14 changes: 14 additions & 0 deletions Tests/LibWeb/Text/input/invalid-transform-interpolation-does-not-crash.html
View file
Open in desktop
@@ -0,0 +1,14 @@
<div id="foo"></div>
<script src="include.js"></script>
<script>
asyncTest(done => {
document.getElementById("foo").animate(
{ transform: ["scale(0)", "scale(0)"] },
{ duration: 1000 },
);
requestAnimationFrame(() => {
println("no crash!");
done();
});
});
</script>
7 changes: 1 addition & 6 deletions Userland/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp
View file
Open in desktop
Expand Up @@ -534,12 +534,7 @@ Optional<StyleProperty> ResolvedCSSStyleDeclaration::property(PropertyID propert
}

if (!m_element->layout_node()) {
auto style_or_error = m_element->document().style_computer().compute_style(const_cast<DOM::Element&>(*m_element));
if (style_or_error.is_error()) {
dbgln("ResolvedCSSStyleDeclaration::property style computer failed");
return {};
}
auto style = style_or_error.release_value();
auto style = m_element->document().style_computer().compute_style(const_cast<DOM::Element&>(*m_element));

// FIXME: This is a stopgap until we implement shorthand -> longhand conversion.
auto value = style->maybe_null_property(property_id);
Expand Down
141 changes: 78 additions & 63 deletions Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
View file
Open in desktop

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions Userland/Libraries/LibWeb/CSS/StyleComputer.h
View file
Open in desktop
Expand Up @@ -52,8 +52,8 @@ class StyleComputer {

NonnullRefPtr<StyleProperties> create_document_style() const;

ErrorOr<NonnullRefPtr<StyleProperties>> compute_style(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type> = {}) const;
ErrorOr<RefPtr<StyleProperties>> compute_pseudo_element_style_if_needed(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>) const;
NonnullRefPtr<StyleProperties> compute_style(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type> = {}) const;
RefPtr<StyleProperties> compute_pseudo_element_style_if_needed(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>) const;

// https://www.w3.org/TR/css-cascade/#origin
enum class CascadeOrigin {
Expand Down Expand Up @@ -87,8 +87,8 @@ class StyleComputer {
class FontLoader;
struct MatchingFontCandidate;

ErrorOr<RefPtr<StyleProperties>> compute_style_impl(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>, ComputeStyleMode) const;
ErrorOr<void> compute_cascaded_values(StyleProperties&, DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>, bool& did_match_any_pseudo_element_rules, ComputeStyleMode) const;
RefPtr<StyleProperties> compute_style_impl(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>, ComputeStyleMode) const;
void compute_cascaded_values(StyleProperties&, DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>, bool& did_match_any_pseudo_element_rules, ComputeStyleMode) const;
static RefPtr<Gfx::FontCascadeList const> find_matching_font_weight_ascending(Vector<MatchingFontCandidate> const& candidates, int target_weight, float font_size_in_pt, bool inclusive);
static RefPtr<Gfx::FontCascadeList const> find_matching_font_weight_descending(Vector<MatchingFontCandidate> const& candidates, int target_weight, float font_size_in_pt, bool inclusive);
RefPtr<Gfx::FontCascadeList const> font_matching_algorithm(FontFaceKey const& key, float font_size_in_pt) const;
Expand Down Expand Up @@ -136,7 +136,7 @@ class StyleComputer {

RuleCache const& rule_cache_for_cascade_origin(CascadeOrigin) const;

ErrorOr<void> collect_animation_into(JS::NonnullGCPtr<Animations::KeyframeEffect> animation, StyleProperties& style_properties) const;
void collect_animation_into(JS::NonnullGCPtr<Animations::KeyframeEffect> animation, StyleProperties& style_properties) const;

OwnPtr<RuleCache> m_author_rule_cache;
OwnPtr<RuleCache> m_user_rule_cache;
Expand Down
3 changes: 1 addition & 2 deletions Userland/Libraries/LibWeb/DOM/Element.cpp
View file
Open in desktop
Expand Up @@ -572,8 +572,7 @@ Element::RequiredInvalidationAfterStyleChange Element::recompute_style()
set_needs_style_update(false);
VERIFY(parent());

// FIXME propagate errors
auto new_computed_css_values = MUST(document().style_computer().compute_style(*this));
auto new_computed_css_values = document().style_computer().compute_style(*this);

// Tables must not inherit -libweb-* values for text-align.
// FIXME: Find the spec for this.
Expand Down
41 changes: 17 additions & 24 deletions Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp
View file
Open in desktop
Expand Up @@ -189,14 +189,14 @@ void TreeBuilder::insert_node_into_inline_or_block_ancestor(Layout::Node& node,
}
}

ErrorOr<void> TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Selector::PseudoElement::Type pseudo_element, AppendOrPrepend mode)
void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Selector::PseudoElement::Type pseudo_element, AppendOrPrepend mode)
{
auto& document = element.document();
auto& style_computer = document.style_computer();

auto pseudo_element_style = TRY(style_computer.compute_pseudo_element_style_if_needed(element, pseudo_element));
auto pseudo_element_style = style_computer.compute_pseudo_element_style_if_needed(element, pseudo_element);
if (!pseudo_element_style)
return {};
return;

auto initial_quote_nesting_level = m_quote_nesting_level;
auto [pseudo_element_content, final_quote_nesting_level] = pseudo_element_style->content(initial_quote_nesting_level);
Expand All @@ -207,11 +207,11 @@ ErrorOr<void> TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element
if (pseudo_element_display.is_none()
|| pseudo_element_content.type == CSS::ContentData::Type::Normal
|| pseudo_element_content.type == CSS::ContentData::Type::None)
return {};
return;

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;

auto generated_for = Node::GeneratedFor::NotGenerated;
if (pseudo_element == CSS::Selector::PseudoElement::Type::Before) {
Expand Down Expand Up @@ -240,8 +240,6 @@ ErrorOr<void> TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element

element.set_pseudo_element_node({}, pseudo_element, pseudo_element_node);
insert_node_into_inline_or_block_ancestor(*pseudo_element_node, pseudo_element_display, mode);

return {};
}

static bool is_ignorable_whitespace(Layout::Node const& node)
Expand Down Expand Up @@ -289,7 +287,7 @@ i32 TreeBuilder::calculate_list_item_index(DOM::Node& dom_node)
return 1;
}

ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& context)
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;
Expand All @@ -311,7 +309,7 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
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) {
return {};
return;
}

auto& document = dom_node.document();
Expand All @@ -326,7 +324,7 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
style = element.computed_css_values();
display = style->display();
if (display.is_none())
return {};
return;
layout_node = element.create_layout_node(*style);
} else if (is<DOM::Document>(dom_node)) {
style = style_computer.create_document_style();
Expand All @@ -338,7 +336,7 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
}

if (!layout_node)
return {};
return;

if (!dom_node.parent_or_shadow_host()) {
m_layout_root = layout_node;
Expand All @@ -354,27 +352,27 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
if (is<DOM::Element>(dom_node) && layout_node->can_have_children()) {
auto& element = static_cast<DOM::Element&>(dom_node);
push_parent(verify_cast<NodeWithStyle>(*layout_node));
TRY(create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::Type::Before, AppendOrPrepend::Prepend));
create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::Type::Before, AppendOrPrepend::Prepend);
pop_parent();
}

if ((dom_node.has_children() || shadow_root) && layout_node->can_have_children()) {
push_parent(verify_cast<NodeWithStyle>(*layout_node));
if (shadow_root) {
for (auto* node = shadow_root->first_child(); node; node = node->next_sibling()) {
TRY(create_layout_tree(*node, context));
create_layout_tree(*node, context);
}
} else {
// This is the same as verify_cast<DOM::ParentNode>(dom_node).for_each_child
for (auto* node = verify_cast<DOM::ParentNode>(dom_node).first_child(); node; node = node->next_sibling())
TRY(create_layout_tree(*node, context));
create_layout_tree(*node, context);
}
pop_parent();
}

if (is<ListItemBox>(*layout_node)) {
auto& element = static_cast<DOM::Element&>(dom_node);
auto marker_style = TRY(style_computer.compute_style(element, CSS::Selector::PseudoElement::Type::Marker));
auto marker_style = style_computer.compute_style(element, CSS::Selector::PseudoElement::Type::Marker);
auto list_item_marker = document.heap().allocate_without_realm<ListItemMarkerBox>(document, layout_node->computed_values().list_style_type(), layout_node->computed_values().list_style_position(), calculate_list_item_index(dom_node), *marker_style);
static_cast<ListItemBox&>(*layout_node).set_marker(list_item_marker);
element.set_pseudo_element_node({}, CSS::Selector::PseudoElement::Type::Marker, list_item_marker);
Expand All @@ -385,11 +383,8 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
auto slottables = static_cast<HTML::HTMLSlotElement&>(dom_node).assigned_nodes_internal();
push_parent(verify_cast<NodeWithStyle>(*layout_node));

for (auto const& slottable : slottables) {
TRY(slottable.visit([&](auto& node) -> ErrorOr<void> {
return create_layout_tree(node, context);
}));
}
for (auto const& slottable : slottables)
slottable.visit([&](auto& node) { create_layout_tree(node, context); });

pop_parent();
}
Expand Down Expand Up @@ -445,11 +440,9 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
if (is<DOM::Element>(dom_node) && layout_node->can_have_children()) {
auto& element = static_cast<DOM::Element&>(dom_node);
push_parent(verify_cast<NodeWithStyle>(*layout_node));
TRY(create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::Type::After, AppendOrPrepend::Append));
create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::Type::After, AppendOrPrepend::Append);
pop_parent();
}

return {};
}

JS::GCPtr<Layout::Node> TreeBuilder::build(DOM::Node& dom_node)
Expand All @@ -458,7 +451,7 @@ JS::GCPtr<Layout::Node> TreeBuilder::build(DOM::Node& dom_node)

Context context;
m_quote_nesting_level = 0;
MUST(create_layout_tree(dom_node, context)); // FIXME propagate errors
create_layout_tree(dom_node, context);

if (auto* root = dom_node.document().layout_node())
fixup_tables(*root);
Expand Down
4 changes: 2 additions & 2 deletions Userland/Libraries/LibWeb/Layout/TreeBuilder.h
View file
Open in desktop
Expand Up @@ -27,7 +27,7 @@ class TreeBuilder {

i32 calculate_list_item_index(DOM::Node&);

ErrorOr<void> create_layout_tree(DOM::Node&, Context&);
void create_layout_tree(DOM::Node&, Context&);

void push_parent(Layout::NodeWithStyle& node) { m_ancestor_stack.append(node); }
void pop_parent() { m_ancestor_stack.take_last(); }
Expand All @@ -49,7 +49,7 @@ class TreeBuilder {
Prepend,
};
void insert_node_into_inline_or_block_ancestor(Layout::Node&, CSS::Display, AppendOrPrepend);
ErrorOr<void> create_pseudo_element_if_needed(DOM::Element&, CSS::Selector::PseudoElement::Type, AppendOrPrepend);
void create_pseudo_element_if_needed(DOM::Element&, CSS::Selector::PseudoElement::Type, AppendOrPrepend);

JS::GCPtr<Layout::Node> m_layout_root;
Vector<JS::NonnullGCPtr<Layout::NodeWithStyle>> m_ancestor_stack;
Expand Down
4 changes: 2 additions & 2 deletions Userland/Services/WebContent/ConnectionFromClient.cpp
View file
Open in desktop
Expand Up @@ -481,7 +481,7 @@ void ConnectionFromClient::debug_request(u64 page_id, ByteString const& request,
for (auto& child : element->children_as_vector())
elements_to_visit.enqueue(child.ptr());
if (element->is_element()) {
auto styles = doc->style_computer().compute_style(*static_cast<Web::DOM::Element*>(element)).release_value_but_fixme_should_propagate_errors();
auto styles = doc->style_computer().compute_style(*static_cast<Web::DOM::Element*>(element));
dbgln("+ Element {}", element->debug_description());
auto& properties = styles->properties();
for (size_t i = 0; i < properties.size(); ++i)
Expand Down Expand Up @@ -704,7 +704,7 @@ void ConnectionFromClient::inspect_dom_node(u64 page_id, i32 node_id, Optional<W
// FIXME: Pseudo-elements only exist as Layout::Nodes, which don't have style information
// in a format we can use. So, we run the StyleComputer again to get the specified
// values, and have to ignore the computed values and custom properties.
auto pseudo_element_style = MUST(page.page().focused_context().active_document()->style_computer().compute_style(element, pseudo_element));
auto pseudo_element_style = page.page().focused_context().active_document()->style_computer().compute_style(element, pseudo_element);
ByteString computed_values = serialize_json(pseudo_element_style);
ByteString resolved_values = "{}";
ByteString custom_properties_json = serialize_custom_properties_json(element, pseudo_element);
Expand Down