Bug 204554 – transition-property is not computed correctly when transition-duration is set to "inherit"

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antoine Quint]

Created attachment 384250 [details]
Test

See the attached test case which amounts to this simple markup:

<div style="transition-duration: 99s; transition-property: margin-left;"><pre style="transition-duration: inherit;"></li></div>

The transition-property for <pre> resolves to the empty string instead of "all".

[Radar WebKit Bug Importer]

<rdar://problem/57458091>

[Antoine Quint]

With the old animation engine, the RenderLayer for the animating <div> is removed after the animation completes and the <div> for the text is positioned.

In the new animation engine, the RenderLayer for the animating <div> still has a RenderLayer after the animation completes.

[Antoine Quint]

This last comment was for another bug, please disregard.

[Antti Koivisto]

I think I see the bug generated style building code. Besides duration this also inherits Animation::animationMode() enum which affects interpretation of Animation::property().


    static void applyInheritTransitionDuration(BuilderState& builderState)
    {
        auto& list = builderState.style().ensureTransitions();
        auto* parentList = builderState.parentStyle().transitions();
        size_t i = 0, parentSize = parentList ? parentList->size() : 0;
        for ( ; i < parentSize && parentList->animation(i).isDurationSet(); ++i) {
            if (list.size() <= i)
                list.append(Animation::create());
            list.animation(i).setDuration(parentList->animation(i).duration());
            list.animation(i).setAnimationMode(parentList->animation(i).animationMode());
        }

        // Reset any remaining animations to not have the property set.
        for ( ; i < list.size(); ++i)
            list.animation(i).clearDuration();
    }

[Antti Koivisto]

Code generation is in makeprop.pl

[Antoine Quint]

Specifically, for this code, in generateAnimationPropertyInheritValueSetter.

[Antoine Quint]

Created attachment 395812 [details]
Patch

[Antoine Quint]

Created attachment 395813 [details]
Patch

[Antti Koivisto]

Comment on attachment 395813 [details]
Patch

Nice. Sad that it doesn't flip any WPTs to PASS.

[Antoine Quint]

(In reply to Antti Koivisto from comment #9)
> Comment on attachment 395813 [details]
> Patch
> 
> Nice. Sad that it doesn't flip any WPTs to PASS.

Not quite, but we pass one additional assertion call in one of the failing tests!

[Antoine Quint]

Committed r259720: <https://trac.webkit.org/changeset/259720>