RESOLVED FIXED 276664
[css-transitions] WPT test css/css-transitions/starting-style-cascade.html has failures
https://bugs.webkit.org/show_bug.cgi?id=276664
Summary [css-transitions] WPT test css/css-transitions/starting-style-cascade.html ha...
Antoine Quint
Reported 2024-07-16 10:03:30 PDT
If I look at https://wpt.fyi/results/css/css-transitions/starting-style-cascade.html, the latest STP (198 at the time of writing) is failing the final subtest "Starting style inheriting from parent's after-change style while ancestor is transitioning": FAIL message: assert_equals: Transition started from parent's after-change style color, inherited from ancestors after-change color expected "rgb(0, 192, 0)" but got "rgb(0, 160, 0)" This subtest is not actually in our checkout, it was added a month ago in https://github.com/web-platform-tests/wpt/commit/401d9af978b784ad6d020ea8c17935ff01002525.
Attachments
Antoine Quint
Comment 1 2024-07-16 10:03:39 PDT
Antoine Quint
Comment 2 2024-07-16 10:05:36 PDT
Antti fixed this for the parent-to-child case in 275061@main but inheritance doesn't work if intermediary elements don't have a transition or don't rely on @starting-style, it appears.
Antoine Quint
Comment 3 2024-07-18 07:40:02 PDT
The issue appears to be that we don't set lastStyleChangeEvent() on elements that aren't animated. Yet, we need this to obtain the after-style change event on the non-animated parent element in the failing test.
Antoine Quint
Comment 4 2024-07-18 08:28:26 PDT
No, that's not the issue. In this case the intermediary, non-animated <div> has its last style change event style set, but the issue is that it contains the inherited value from the parent which matches the current animated state, not the final state of the transition. We somehow need to be able to reconstitute that value.
Antoine Quint
Comment 5 2024-07-18 08:57:11 PDT
Perhaps what we need to do here is not to use the `resolvedStyle.style` as the style to store as the last style change event style in Style::TreeResolver::createAnimatedElementUpdate(), but rather create that style by inheriting from the parent's last style change event style. That way the last style change event style would become the after-change style. We'll have to see whether that causes regression. We may have to store both otherwise, but I worry this could cause a perf or memory regression.
Antoine Quint
Comment 6 2024-07-18 13:05:20 PDT
I got it to work quickly, and it does work. My current work-in-progress patch breaks a host of pseudo-element-related tests, so I'll need to figure this out first and then see if we need to introduce the notion of an after-change style on top of the last style change event style.
Antoine Quint
Comment 7 2024-07-19 09:49:24 PDT
I made good progress on this in https://github.com/graouts/WebKit/tree/after-change-style but have not got something working without regressing other tests. I'll resume work on this later in August.
Antoine Quint
Comment 8 2024-09-03 06:49:44 PDT
The failing test is being imported in bug 279058.
Antti Koivisto
Comment 9 2024-09-10 01:09:35 PDT
EWS
Comment 10 2024-09-10 10:05:49 PDT
Committed 283423@main (826e5996a267): <https://commits.webkit.org/283423@main> Reviewed commits have been landed. Closing PR #33382 and removing active labels.
EWS
Comment 11 2024-09-11 14:21:41 PDT
Committed 283286.13@safari-7620-branch (1c106d4bb45e): <https://commits.webkit.org/283286.13@safari-7620-branch> Reviewed commits have been landed. Closing PR #1727 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.