Skip to content

Commit 4049cce

Browse files
kalenikaliaksandrawesomekling
authored andcommitted
LibWeb: Add slots for pseudo-elements animation cache in Animatable
Fixes the bug when animation does not run at all if an element has a pseudo-element, because both of them use the same cache.
1 parent 6a19cff commit 4049cce

File tree

3 files changed

+51
-16
lines changed

3 files changed

+51
-16
lines changed

Userland/Libraries/LibWeb/Animations/Animatable.cpp

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,42 @@ void Animatable::disassociate_with_animation(JS::NonnullGCPtr<Animation> animati
102102
void Animatable::visit_edges(JS::Cell::Visitor& visitor)
103103
{
104104
visitor.visit(m_associated_animations);
105-
visitor.visit(m_cached_animation_name_source);
106-
visitor.visit(m_cached_animation_name_animation);
105+
for (auto const& cached_animation_source : m_cached_animation_name_source)
106+
visitor.visit(cached_animation_source);
107+
for (auto const& cached_animation_name : m_cached_animation_name_animation)
108+
visitor.visit(cached_animation_name);
109+
}
110+
111+
JS::GCPtr<CSS::CSSStyleDeclaration const> Animatable::cached_animation_name_source(Optional<CSS::Selector::PseudoElement::Type> pseudo_element) const
112+
{
113+
if (pseudo_element.has_value())
114+
return m_cached_animation_name_source[to_underlying(pseudo_element.value()) + 1];
115+
return m_cached_animation_name_source[0];
116+
}
117+
118+
void Animatable::set_cached_animation_name_source(JS::GCPtr<CSS::CSSStyleDeclaration const> value, Optional<CSS::Selector::PseudoElement::Type> pseudo_element)
119+
{
120+
if (pseudo_element.has_value()) {
121+
m_cached_animation_name_source[to_underlying(pseudo_element.value()) + 1] = value;
122+
} else {
123+
m_cached_animation_name_source[0] = value;
124+
}
125+
}
126+
127+
JS::GCPtr<Animations::Animation> Animatable::cached_animation_name_animation(Optional<CSS::Selector::PseudoElement::Type> pseudo_element) const
128+
{
129+
if (pseudo_element.has_value())
130+
return m_cached_animation_name_animation[to_underlying(pseudo_element.value()) + 1];
131+
return m_cached_animation_name_animation[0];
132+
}
133+
134+
void Animatable::set_cached_animation_name_animation(JS::GCPtr<Animations::Animation> value, Optional<CSS::Selector::PseudoElement::Type> pseudo_element)
135+
{
136+
if (pseudo_element.has_value()) {
137+
m_cached_animation_name_animation[to_underlying(pseudo_element.value()) + 1] = value;
138+
} else {
139+
m_cached_animation_name_animation[0] = value;
140+
}
107141
}
108142

109143
}

Userland/Libraries/LibWeb/Animations/Animatable.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,20 +33,21 @@ class Animatable {
3333
void associate_with_animation(JS::NonnullGCPtr<Animation>);
3434
void disassociate_with_animation(JS::NonnullGCPtr<Animation>);
3535

36-
JS::GCPtr<CSS::CSSStyleDeclaration const> cached_animation_name_source() const { return m_cached_animation_name_source; }
37-
void set_cached_animation_name_source(JS::GCPtr<CSS::CSSStyleDeclaration const> value) { m_cached_animation_name_source = value; }
36+
JS::GCPtr<CSS::CSSStyleDeclaration const> cached_animation_name_source(Optional<CSS::Selector::PseudoElement::Type>) const;
37+
void set_cached_animation_name_source(JS::GCPtr<CSS::CSSStyleDeclaration const> value, Optional<CSS::Selector::PseudoElement::Type>);
3838

39-
JS::GCPtr<Animations::Animation> cached_animation_name_animation() const { return m_cached_animation_name_animation; }
40-
void set_cached_animation_name_animation(JS::GCPtr<Animations::Animation> value) { m_cached_animation_name_animation = value; }
39+
JS::GCPtr<Animations::Animation> cached_animation_name_animation(Optional<CSS::Selector::PseudoElement::Type>) const;
40+
void set_cached_animation_name_animation(JS::GCPtr<Animations::Animation> value, Optional<CSS::Selector::PseudoElement::Type>);
4141

4242
protected:
4343
void visit_edges(JS::Cell::Visitor&);
4444

4545
private:
4646
Vector<JS::NonnullGCPtr<Animation>> m_associated_animations;
4747
bool m_is_sorted_by_composite_order { true };
48-
JS::GCPtr<CSS::CSSStyleDeclaration const> m_cached_animation_name_source;
49-
JS::GCPtr<Animations::Animation> m_cached_animation_name_animation;
48+
49+
Array<JS::GCPtr<CSS::CSSStyleDeclaration const>, to_underlying(CSS::Selector::PseudoElement::Type::KnownPseudoElementCount) + 1> m_cached_animation_name_source;
50+
Array<JS::GCPtr<Animations::Animation>, to_underlying(CSS::Selector::PseudoElement::Type::KnownPseudoElementCount) + 1> m_cached_animation_name_animation;
5051
};
5152

5253
}

Userland/Libraries/LibWeb/CSS/StyleComputer.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1774,11 +1774,11 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element
17741774
if (auto source_declaration = style.property_source_declaration(PropertyID::AnimationName); source_declaration) {
17751775
auto& realm = element.realm();
17761776

1777-
if (source_declaration != element.cached_animation_name_source()) {
1777+
if (source_declaration != element.cached_animation_name_source(pseudo_element)) {
17781778
// This animation name is new, so we need to create a new animation for it.
1779-
if (auto existing_animation = element.cached_animation_name_animation())
1779+
if (auto existing_animation = element.cached_animation_name_animation(pseudo_element))
17801780
existing_animation->cancel(Animations::Animation::ShouldInvalidate::No);
1781-
element.set_cached_animation_name_source(source_declaration);
1781+
element.set_cached_animation_name_source(source_declaration, pseudo_element);
17821782

17831783
auto effect = Animations::KeyframeEffect::create(realm);
17841784
auto animation = CSSAnimation::create(realm);
@@ -1795,21 +1795,21 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element
17951795
effect->set_key_frame_set(keyframe_set.value());
17961796

17971797
effect->set_target(&element);
1798-
element.set_cached_animation_name_animation(animation);
1798+
element.set_cached_animation_name_animation(animation, pseudo_element);
17991799

18001800
HTML::TemporaryExecutionContext context(m_document->relevant_settings_object());
18011801
animation->play().release_value_but_fixme_should_propagate_errors();
18021802
} else {
18031803
// The animation hasn't changed, but some properties of the animation may have
1804-
apply_animation_properties(m_document, style, *element.cached_animation_name_animation());
1804+
apply_animation_properties(m_document, style, *element.cached_animation_name_animation(pseudo_element));
18051805
}
18061806
}
18071807
} else {
18081808
// If the element had an existing animation, cancel it
1809-
if (auto existing_animation = element.cached_animation_name_animation()) {
1809+
if (auto existing_animation = element.cached_animation_name_animation(pseudo_element)) {
18101810
existing_animation->cancel(Animations::Animation::ShouldInvalidate::No);
1811-
element.set_cached_animation_name_animation({});
1812-
element.set_cached_animation_name_source({});
1811+
element.set_cached_animation_name_animation({}, pseudo_element);
1812+
element.set_cached_animation_name_source({}, pseudo_element);
18131813
}
18141814
}
18151815

0 commit comments

Comments
 (0)