Stowage/ladybird
2
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-12-07 21:59:54 +00:00
Code Issues Projects Releases Packages Wiki Activity

LibWeb: Ensure registered transitions are reflective of properties

Browse source 
Previously we would only update these if:
a) We had a cascaded value for `transition-property`
b) The source of that cascaded value had changed since we last
   registered transitions

This meant that there were a lot of changes we didn't apply:
 - Changes exclusively to properties other than `transition-property`
   (e.g. `transition-duration`, `transition-behavior`, etc)
 - Removing the `transition-property` property
 - Updating the `transition-property` property in a way that didn't
   change it's source (e.g. setting it within inline-style)

Unfortunately this does mean that we now register transitions for all
properties on most elements since "all" is the initial value for
"transition-property" which isn't great for performance, but that can be
looked at in later commits.
This commit is contained in:
Callum Law 2025-11-18 22:15:15 +13:00 • committed by Jelle Raaijmakers
parent 46abc0e8e2
commit b2b889e1da
Notes: github-actions[bot] 2025-11-23 08:44:36 +00:00
13 changed files with 98 additions and 45 deletions
Download patch file Download diff file Expand all files Collapse all files

18
Libraries/LibWeb/Animations/Animatable.cpp
View file

@ -22,7 +22,6 @@ namespace Web::Animations {
struct Animatable::Transition {
HashMap<CSS::PropertyID, size_t> transition_attribute_indices;
Vector<TransitionAttributes> transition_attributes;
GC::Ptr<CSS::CSSStyleDeclaration const> cached_transition_property_source;
HashMap<CSS::PropertyID, GC::Ref<CSS::CSSTransition>> associated_transitions;
};
@ -255,7 +254,6 @@ void Animatable::visit_edges(JS::Cell::Visitor& visitor)
for (auto const& transition : impl.transitions) {
if (transition) {
visitor.visit(transition->cached_transition_property_source);
visitor.visit(transition->associated_transitions);
}
}
@ -291,22 +289,6 @@ HashMap<FlyString, GC::Ref<Animation>>* Animatable::css_defined_animations(Optio
return impl.css_defined_animations[index];
}
GC::Ptr<CSS::CSSStyleDeclaration const> Animatable::cached_transition_property_source(Optional<CSS::PseudoElement> pseudo_element) const
{
auto* maybe_transition = ensure_transition(pseudo_element);
if (!maybe_transition)
return {};
return maybe_transition->cached_transition_property_source;
}
void Animatable::set_cached_transition_property_source(Optional<CSS::PseudoElement> pseudo_element, GC::Ptr<CSS::CSSStyleDeclaration const> value)
{
auto* maybe_transition = ensure_transition(pseudo_element);
if (!maybe_transition)
return;
maybe_transition->cached_transition_property_source = value;
}
Animatable::Impl& Animatable::ensure_impl() const
{
if (!m_impl)

3
Libraries/LibWeb/Animations/Animatable.h
View file

@ -58,9 +58,6 @@ public:
void add_css_animation(FlyString name, Optional<CSS::PseudoElement>, GC::Ref<Animation>);
void remove_css_animation(FlyString name, Optional<CSS::PseudoElement>);
GC::Ptr<CSS::CSSStyleDeclaration const> cached_transition_property_source(Optional<CSS::PseudoElement>) const;
void set_cached_transition_property_source(Optional<CSS::PseudoElement>, GC::Ptr<CSS::CSSStyleDeclaration const> value);
void add_transitioned_properties(Optional<CSS::PseudoElement>, Vector<Vector<CSS::PropertyID>> properties, CSS::StyleValueVector delays, CSS::StyleValueVector durations, CSS::StyleValueVector timing_functions, CSS::StyleValueVector transition_behaviors);
Vector<CSS::PropertyID> property_ids_with_matching_transition_property_entry(Optional<CSS::PseudoElement>) const;
Optional<TransitionAttributes const&> property_transition_attributes(Optional<CSS::PseudoElement>, CSS::PropertyID) const;

1
Libraries/LibWeb/CSS/ComputedProperties.cpp
View file

@ -55,7 +55,6 @@ ComputedProperties::~ComputedProperties() = default;
void ComputedProperties::visit_edges(Visitor& visitor)
{
Base::visit_edges(visitor);
visitor.visit(m_transition_property_source);
}
bool ComputedProperties::is_property_important(PropertyID property_id) const

5
Libraries/LibWeb/CSS/ComputedProperties.h
View file

@ -70,9 +70,6 @@ public:
StyleValue const& property(PropertyID, WithAnimationsApplied = WithAnimationsApplied::Yes) const;
void revert_property(PropertyID, ComputedProperties const& style_for_revert);
GC::Ptr<CSSStyleDeclaration const> transition_property_source() const { return m_transition_property_source; }
void set_transition_property_source(GC::Ptr<CSSStyleDeclaration const> declaration) { m_transition_property_source = declaration; }
Size size_value(PropertyID) const;
[[nodiscard]] Variant<LengthPercentage, NormalGap> gap_value(PropertyID) const;
Length length(PropertyID) const;
@ -291,8 +288,6 @@ private:
Vector<ShadowData> shadow(PropertyID, Layout::Node const&) const;
Position position_value(PropertyID) const;
GC::Ptr<CSSStyleDeclaration const> m_transition_property_source;
Array<RefPtr<StyleValue const>, number_of_longhand_properties> m_property_values;
Array<u8, ceil_div(number_of_longhand_properties, 8uz)> m_property_important {};
Array<u8, ceil_div(number_of_longhand_properties, 8uz)> m_property_inherited {};

14
Libraries/LibWeb/CSS/StyleComputer.cpp
View file

@ -1221,19 +1221,15 @@ static void apply_dimension_attribute(CascadedProperties& cascaded_properties, D
static void compute_transitioned_properties(ComputedProperties const& style, DOM::AbstractElement abstract_element)
{
auto const source_declaration = style.transition_property_source();
if (!source_declaration)
return;
// FIXME: For now we don't bother registering transitions on the first computation since they can't run (because
// there is nothing to transition from) but this will change once we implement @starting-style
if (!abstract_element.computed_properties())
return;
// FIXME: Add transition helpers on AbstractElement.
auto& element = abstract_element.element();
auto pseudo_element = abstract_element.pseudo_element();
if (source_declaration == element.cached_transition_property_source(pseudo_element))
return;
// Reparse this transition property
element.clear_registered_transitions(pseudo_element);
element.set_cached_transition_property_source(pseudo_element, *source_declaration);
auto coordinated_transition_list = style.assemble_coordinated_value_list(
PropertyID::TransitionProperty,
@ -2541,10 +2537,6 @@ GC::Ref<ComputedProperties> StyleComputer::compute_properties(DOM::AbstractEleme
computed_style->set_property(property_id, value.release_nonnull(), inherited, cascaded_properties.is_property_important(property_id) ? Important::Yes : Important::No);
if (animated_value.has_value())
computed_style->set_animated_property(property_id, animated_value.value(), inherited);
if (property_id == PropertyID::TransitionProperty) {
computed_style->set_transition_property_source(cascaded_properties.property_source(property_id));
}
}
// Compute the value of custom properties

1
Tests/LibWeb/Text/expected/css/transition-properties-updated-on-duration-change.txt Normal file
View file

@ -0,0 +1 @@
2000

1
Tests/LibWeb/Text/expected/css/transition-properties-updated-on-property-name-changed-in-same-css-style-properties.txt Normal file
View file

@ -0,0 +1 @@
1

8
Tests/LibWeb/Text/expected/wpt-import/css/css-pseudo/parsing/marker-supported-properties-in-animation.txt
View file

@ -2,8 +2,8 @@ Harness status: OK
Found 96 tests
39 Pass
57 Fail
41 Pass
55 Fail
Pass Animation of font in ::marker
Pass Animation of font-family in ::marker
Fail Animation of font-feature-settings in ::marker
@ -59,7 +59,7 @@ Pass Transition of font-kerning in ::marker
Fail Transition of font-size in ::marker
Fail Transition of font-size-adjust in ::marker
Fail Transition of font-stretch in ::marker
Fail Transition of font-style in ::marker
Pass Transition of font-style in ::marker
Fail Transition of font-synthesis in ::marker
Fail Transition of font-synthesis-small-caps in ::marker
Fail Transition of font-synthesis-style in ::marker
@ -91,7 +91,7 @@ Fail Transition of text-emphasis in ::marker
Fail Transition of text-emphasis-color in ::marker
Fail Transition of text-emphasis-position in ::marker
Fail Transition of text-emphasis-style in ::marker
Fail Transition of text-shadow in ::marker
Pass Transition of text-shadow in ::marker
Pass Transition of cursor in ::marker
Fail Transition of display in ::marker
Fail Transition of position in ::marker

5
Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/CSSTransition-ready.tentative.txt
View file

@ -2,7 +2,6 @@ Harness status: OK
Found 2 tests
1 Pass
1 Fail
Fail ready promise is rejected when a transition is canceled by updating transition-property
2 Pass
Pass ready promise is rejected when a transition is canceled by updating transition-property
Pass ready promise is rejected when a transition is canceled by changing the transition property to something not interpolable

6
Tests/LibWeb/Text/expected/wpt-import/css/css-transitions/transition-remove-and-change-immediate.txt Normal file
View file

@ -0,0 +1,6 @@
Harness status: OK
Found 1 tests
1 Pass
Pass Removing transition and changing width applies change immediately

15
Tests/LibWeb/Text/input/css/transition-properties-updated-on-duration-change.html Normal file
View file

@ -0,0 +1,15 @@
<!DOCTYPE html>
<div id="foo" style="transition: top 1s; top: 0px"></div>
<script src="../include.js"></script>
<script>
test(() => {
// Transitions properties aren't set until the second computation as a performance optimization so
// force a recomputation
foo.style.left = "0px";
getComputedStyle(foo).left;
foo.style.transitionDuration = "2s";
foo.style.top = "10px";
println(document.getAnimations()[0].effect.getComputedTiming().duration);
});
</script>

15
Tests/LibWeb/Text/input/css/transition-properties-updated-on-property-name-changed-in-same-css-style-properties.html Normal file
View file

@ -0,0 +1,15 @@
<!DOCTYPE html>
<div id="foo" style="transition: none 1s; top: 0px"></div>
<script src="../include.js"></script>
<script>
test(() => {
// Transitions aren't registered until the second computation as a performance optimization so
// force a recomputation
foo.style.left = "0px";
getComputedStyle(foo).left;
foo.style.transitionProperty = "top";
foo.style.top = "10px";
println(document.getAnimations().length);
});
</script>

51
Tests/LibWeb/Text/input/wpt-import/css/css-transitions/transition-remove-and-change-immediate.html Normal file
View file

@ -0,0 +1,51 @@
<!doctype html>
<html>
<head>
<meta charset=utf-8>
<title>CSS Transitions Test: Removing transition and changing width applies change immediately</title>
<meta name="assert" content="When a transition is removed and a width is changed after a previous transition completes, the change is immediate.">
<link rel="help" href="https://drafts.csswg.org/css-transitions/#starting">
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="./support/helper.js"></script>
</head>
<body>
<div id="log"></div>
<script>
promise_test(async t => {
const div = addDiv(t, {
style: 'transition: width 0.02s; width: 0px;'
});
// Flush initial state
getComputedStyle(div).width;
div.style.width = '100px';
// Wait for transition to complete
await waitForTransitionEnd(div);
// Verify the width after first transition
const afterFirst = getComputedStyle(div).width;
assert_equals(afterFirst, '100px', 'width should be 100px after first transition');
// Set width back to 0 and remove transition
div.style.width = '0px';
div.style.transition = '';
// Force layout update to ensure style computation
const afterSecond = getComputedStyle(div).width;
// Check width is immediately updated
assert_equals(
afterSecond,
'0px',
'width should reset to 0 immediately'
);
}, 'Removing transition and changing width applies change immediately');
</script>
</body>
</html>