Bug 220862 - REGRESSION (r271584): Hovering slowly over and out of "Top 100" items on liberation.fr does not restore animated state
Summary: REGRESSION (r271584): Hovering slowly over and out of "Top 100" items on libe...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-01-22 08:52 PST by Antoine Quint
Modified: 2021-01-26 20:12 PST (History)
11 users (show)

See Also:


Attachments
Screen recording (4.81 MB, video/quicktime)
2021-01-22 08:52 PST, Antoine Quint
no flags Details
patch (20.09 KB, patch)
2021-01-26 02:17 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (20.25 KB, patch)
2021-01-26 04:14 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (20.90 KB, patch)
2021-01-26 06:08 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2021-01-22 08:52:08 PST
Created attachment 418151 [details]
Screen recording

Steps to reproduce:

1. Open https://www.liberation.fr/
2. Wait for the page to finish loading and for only 5 items to show under "Top 100" near the center of the page
3. Hover slowly over those 5 items

Notice how, as in the attached screen recording, the items don't revert back to their pre-animated state after hovering out.

I only managed to reduce the failed to this range:

http://trac.webkit.org/log/trunk/?mode=follow_copy&rev=271601&stop_rev=271568

Out of this range, r271584, the fix for bug 220711, seems the most likely since it's related to :hover.
Comment 1 Radar WebKit Bug Importer 2021-01-22 08:52:51 PST
<rdar://problem/73501684>
Comment 2 Antti Koivisto 2021-01-25 10:22:42 PST
Ah yes, there is a logic error in the optimization where we fail to invalidate descendants losing hover/active in some complex cases.
Comment 3 Antti Koivisto 2021-01-26 02:17:41 PST
Created attachment 418384 [details]
patch
Comment 4 Antti Koivisto 2021-01-26 04:14:10 PST
Created attachment 418392 [details]
patch
Comment 5 Antti Koivisto 2021-01-26 06:08:45 PST
Created attachment 418409 [details]
patch
Comment 6 Simon Fraser (smfr) 2021-01-26 11:33:35 PST
Comment on attachment 418409 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418409&action=review

> Source/WebCore/dom/Document.cpp:7234
> +        Style::PseudoClassChangeInvalidation styleInvalidation(*elementsToClearActive.last(), CSSSelector::PseudoClassActive, Style::InvalidationScope::Descendants);
> +        for (auto& element : elementsToClearActive)
> +            element->setActive(false, false, Style::InvalidationScope::SelfChildrenAndSiblings);

Maybe share these almost-duplicate chunks with some lambdas?
Comment 7 Antti Koivisto 2021-01-26 11:48:55 PST
> Maybe share these almost-duplicate chunks with some lambdas?

It seemed to end up looking messy since there is a bunch of varying parameters. But I'll take another look.
Comment 8 EWS 2021-01-26 18:38:20 PST
Committed r271930: <https://trac.webkit.org/changeset/271930>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418409 [details].
Comment 9 Simon Fraser (smfr) 2021-01-26 20:12:30 PST
Landed early because it fixed other bugs that were blocking internal Apple work.