Bug 220862

Summary: REGRESSION (r271584): Hovering slowly over and out of "Top 100" items on liberation.fr does not restore animated state
Product: WebKit Reporter: Antoine Quint <graouts>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, cmarcelo, esprehn+autocc, ews-watchlist, gyuyoung.kim, kangil.han, koivisto, mifenton, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Screen recording
none
patch
none
patch
none
patch none

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.