[webkit-changes] [WebKit/WebKit] d08faf: REGRESSION (276531 at main): Content flash when expan...

[Antoine Quint]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: d08faf9fab62fcbe230979774bacbdc53684e816
      https://github.com/WebKit/WebKit/commit/d08faf9fab62fcbe230979774bacbdc53684e816
  Author: Antoine Quint <graouts at webkit.org>
  Date:   2024-07-11 (Thu, 11 Jul 2024)

  Changed paths:
    A LayoutTests/imported/w3c/web-platform-tests/css/css-animations/display-none-prevents-starting-in-subtree-expected.txt
    A LayoutTests/imported/w3c/web-platform-tests/css/css-animations/display-none-prevents-starting-in-subtree.html
    M Source/WebCore/rendering/updating/RenderTreeUpdater.cpp
    M Source/WebCore/style/StyleTreeResolver.cpp
    M Source/WebCore/style/StyleTreeResolver.h
    M Source/WebCore/style/Styleable.cpp
    M Source/WebCore/style/Styleable.h

  Log Message:
  -----------
  REGRESSION (276531 at main): Content flash when expanding image viewer on eBay page
https://bugs.webkit.org/show_bug.cgi?id=275739
rdar://128178476

Reviewed by Alan Baradlay.

Listings on eBay can display a photo gallery which is contained within a subtree where
the `hidden` presentation attribute is toggled to show and hide the gallery. The `hidden`
attribute really governs whether `display: none` is applied. In that subtree, a forwards-filling
`opacity` CSS Animation is applied.

Prior to `276531 at main`, when `Style::TreeResolver::resolveElement()` would be called under
`Style::TreeResolver::resolveComposedTree()` for an element with `display: none`, an empty
`ElementUpdate` would be returned and `resolveComposedTree()` would not process its children.

In `276531 at main`, we modified `resolveElement()` such that this behavior would not only apply
to elements with a newly-set `display: none` style associated with them, excluding those with
a cached `display: none` style. This was done in the static function `affectsRenderedSubtree()`.

This meant that now, if an element within a `display: none` subtree had its style modified,
`resolveElement()` would be called for that entire subtree when it previously wouldn't have.
As a result, `createAnimatedElementUpdate()` would now be called and since this method had
only ever been called for elements not contained within a `display: none` subtree, it would
not consider that possibility and start CSS Animations in that subtree.

This is what caused the eBay regression because the forwards-filling animation meant to fade
the photo gallery in would be reapplied soon after the photo gallery was hidden after an
additional style update, and when that photo gallery would become visible again, the animation
was already in effect and the gallery did not have the expected computed `opacity` value.

To fix this, we capture a new `isInDisplayNoneTree` state on the `Parent` objects created as
we iterate through the composed tree in `resolveComposedTree()`. That new member identifies
whether that element or a parent has `display: none` set on it. Now we can pass that state
down to `createAnimatedElementUpdate()` and ultimately to `Styleable::updateCSSAnimations()`
and ultimately neglect to start new CSS Animations when the element in question is contained
within a `display: none` subtree.

We also removed the `affectsRenderedSubtree()` changed introduced in `276531 at main` which simply
did not make much sense, and to ensure that elements with `display: none` that had their style
updated correctly updated their cached `display: none` style, we now return a non-empty
`ElementUpdate` and add that `ElementUpdate` to `m_update` in `resolveComposedTree()`.

Finally, we add a new WPT test that reduces the eBay behavior.

* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/display-none-prevents-starting-in-subtree-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/display-none-prevents-starting-in-subtree.html: Added.
* Source/WebCore/rendering/updating/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::updateElementRenderer):
* Source/WebCore/style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::Parent::Parent):
(WebCore::Style::affectsRenderedSubtree):
(WebCore::Style::TreeResolver::resolveElement):
(WebCore::Style::TreeResolver::resolvePseudoElement):
(WebCore::Style::TreeResolver::createAnimatedElementUpdate):
(WebCore::Style::TreeResolver::pushParent):
(WebCore::Style::TreeResolver::resolveComposedTree):
* Source/WebCore/style/StyleTreeResolver.h:
* Source/WebCore/style/Styleable.cpp:
(WebCore::Styleable::updateCSSAnimations const):
* Source/WebCore/style/Styleable.h:

Canonical link: https://commits.webkit.org/280876@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications