Summary: | css/css-transitions/pseudo-elements-002.html WPT is a failure | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||||||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | dino, esprehn+autocc, ews-watchlist, glenn, graouts, gyuyoung.kim, koivisto, macpherson, menard, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=235158 | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 235130 | ||||||||||||||
Attachments: |
|
Description
Antoine Quint
2022-01-07 14:29:22 PST
Adding a call to `outer.getAnimations({ subtree: true });` right after the call to `outer.className = "flex";` makes the test pass since it forces a style update which otherwise doesn't quite happen just by querying the computed style. But things are still wrong because the transition is removed as the pseudo-element is destroyed and recreated and the returned value for the getAnimations() call is empty while other browsers would return the generated CSSTransition. In Document::resolveStyle(), after we're done with the call to Style::TreeResolver::resolve() and have obtained our styleUpdate, we set the m_inStyleRecalc flag to false and call `updateRenderTree(WTFMove(styleUpdate))` and this is under there that we remove the pseudo-element and recreate it. I guess we need to either: 1. avoid the pseudo-element teardown/rebuild, 2. avoid clearing the animation-related data structures upon its removal 3. be able to restore the animation-related data structures on it after recreation Well, actually, there remains the issue that the transition isn't created as the computed style is requested after setting the "flex" class on the parent, so there may be several issues here. Created attachment 448912 [details]
Simplified failing testcase (height)
Attaching a simplified standalone testcase which shows that we fail to call createAnimatedElementUpdate() when changing the class on the parent. The style is correctly updated (we get 300px) but the transition is not started.
Created attachment 448913 [details]
Simplified working testcase (color)
The same test using the "color" property works.
So for the simplified height case, when ComputedStyleExtractor::propertyValue() is called we call into updateStyleIfNeededForProperty() and hasValidStyle() is true within that function so we do _not_ end up calling document.updateStyleIfNeeded(). In the simplified color case, we return false for hasValidStyleProperty due to this clause: if ((isInherited || maybeExplicitlyInherited) && ancestor.styleValidity() == Style::Validity::ElementInvalid) return false; … and end up calling document.updateStyleIfNeeded() which then lets the transition start. If I change the "height" test to not involve pseudo-elements, then hasValidStyleProperty() returns false due to: if (element.styleValidity() != Style::Validity::Valid) return false; Created attachment 448922 [details]
Patch
Created attachment 448933 [details]
Patch for landing
Created attachment 448943 [details]
Patch for landing
Committed r287926 (245959@trunk): <https://commits.webkit.org/245959@trunk> We'll clean up the added use of PseudoElement in bug 235158. |