Skip to content
Permalink
Browse files
Animation.commitStyles() doesn't change "style" attribute for individ…
…ual CSS transform properties

https://bugs.webkit.org/show_bug.cgi?id=247245
rdar://102014524

Reviewed by Antti Koivisto.

When commitStyles() is called a null renderer is passed down to ComputedStyleExtractor::valueForPropertyInStyle().
This would make the various functions computing values for individual transform properties return "none" since
those would check for a non-null, non-inline renderer. We change those checks to only check for an inline renderer
to return "none".

We also had to make a change to how we call MutableStyleProperties::setProperty() in commitStyles() to provide an
explicit CSSParserContext created for the styled element's document such that we have valid settings. Otherwise,
properties governed by a runtime flag, such as the individual transform properties, would show as disabled and
we would hit an ASSERT_NOT_REACHED under MutableStyleProperties::setProperty().

Finally, we add a new WPT test to check that commitStyles() correctly commits individual transform properties
to the inline style.

* LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/commitStyles-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/commitStyles.html:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::commitStyles):
* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::computedTranslate):
(WebCore::computedScale):
(WebCore::computedRotate):
(WebCore::rendererCanBeTransformed): Deleted.

Canonical link: https://commits.webkit.org/256728@main
  • Loading branch information
graouts committed Nov 16, 2022
1 parent da44a79 commit c148a252e8ff4bb32d7537cb35638741cf6fe9d5
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 10 deletions.
@@ -1,5 +1,6 @@

PASS Commits styles
PASS Commits styles for individual transform properties
PASS Commits styles for an animation that has been removed
PASS Commits shorthand styles
PASS Commits logical properties
@@ -42,6 +42,30 @@
assert_numeric_style_equals(getComputedStyle(div).opacity, 0.2);
}, 'Commits styles');

test(t => {
const div = createDiv(t);
div.style.translate = '100px';
div.style.rotate = '45deg';
div.style.scale = '2';

const animation = div.animate(
{ translate: '200px',
rotate: '90deg',
scale: 3 },
{ duration: 1, fill: 'forwards' }
);
animation.finish();

animation.commitStyles();

// Cancel the animation so we can inspect the underlying style
animation.cancel();

assert_equals(getComputedStyle(div).translate, '200px');
assert_equals(getComputedStyle(div).rotate, '90deg');
assert_numeric_style_equals(getComputedStyle(div).scale, 3);
}, 'Commits styles for individual transform properties');

promise_test(async t => {
const div = createDiv(t);
div.style.opacity = '0.1';
@@ -1578,7 +1578,7 @@ ExceptionOr<void> WebAnimation::commitStyles()
return WTF::switchOn(property,
[&] (CSSPropertyID propertyId) {
if (auto cssValue = computedStyleExtractor.valueForPropertyInStyle(*animatedStyle, propertyId, nullptr, ComputedStyleExtractor::PropertyValueType::Computed))
return inlineStyle->setProperty(propertyId, cssValue->cssText(), false);
return inlineStyle->setProperty(propertyId, cssValue->cssText(), false, { styledElement.document() });
return false;
},
[&] (AtomString customProperty) {
@@ -725,12 +725,6 @@ static Ref<CSSFunctionValue> matrixTransformValue(const TransformationMatrix& tr
return transformValue.releaseNonNull();
}

static bool rendererCanBeTransformed(RenderObject* renderer)
{
// Inline renderers do not support transforms.
return renderer && !is<RenderInline>(*renderer);
}

static Ref<CSSValue> computedTransform(RenderElement* renderer, const RenderStyle& style, ComputedStyleExtractor::PropertyValueType valueType)
{
auto& cssValuePool = CSSValuePool::singleton();
@@ -899,7 +893,7 @@ static Ref<CSSValue> computedTransform(RenderElement* renderer, const RenderStyl
static Ref<CSSValue> computedTranslate(RenderObject* renderer, const RenderStyle& style)
{
auto* translate = style.translate();
if (!translate || !rendererCanBeTransformed(renderer) || translate->isIdentity())
if (!translate || is<RenderInline>(renderer) || translate->isIdentity())
return CSSValuePool::singleton().createIdentifierValue(CSSValueNone);

auto list = CSSValueList::createSpaceSeparated();
@@ -922,7 +916,7 @@ static Ref<CSSValue> computedScale(RenderObject* renderer, const RenderStyle& st
{
auto* scale = style.scale();
auto& cssValuePool = CSSValuePool::singleton();
if (!scale || !rendererCanBeTransformed(renderer) || scale->isIdentity())
if (!scale || is<RenderInline>(renderer) || scale->isIdentity())
return cssValuePool.createIdentifierValue(CSSValueNone);

auto list = CSSValueList::createSpaceSeparated();
@@ -939,7 +933,7 @@ static Ref<CSSValue> computedRotate(RenderObject* renderer, const RenderStyle& s
{
auto* rotate = style.rotate();
auto& cssValuePool = CSSValuePool::singleton();
if (!rotate || !rendererCanBeTransformed(renderer) || rotate->isIdentity())
if (!rotate || is<RenderInline>(renderer) || rotate->isIdentity())
return cssValuePool.createIdentifierValue(CSSValueNone);

if (!rotate->is3DOperation() || (!rotate->x() && !rotate->y() && rotate->z()))

0 comments on commit c148a25

Please sign in to comment.