Bug 218741

Summary: [Web Animations] KeyframeEffect.pseudoElement does not return a valid string when targeting ::marker or ::first-letter
Product: WebKit Reporter: Antoine Quint <graouts>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, dino, esprehn+autocc, ews-watchlist, kangil.han, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Antoine Quint 2020-11-10 01:14:11 PST
[Web Animations] :KeyframeEffect.pseudoElement does not return a valid string when targeting ::marker or ::first-letter
Comment 1 Radar WebKit Bug Importer 2020-11-10 01:14:57 PST
<rdar://problem/71229846>
Comment 2 Antoine Quint 2020-11-10 01:17:10 PST
Created attachment 413677 [details]
Patch
Comment 3 Antoine Quint 2020-11-10 01:17:15 PST
<rdar://problem/71229846>
Comment 4 Dean Jackson 2020-11-10 01:25:35 PST
Comment on attachment 413677 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        We used to use PseudoElement::pseudoElementNameForEvents() to go from PseudoId to a String, but PseudoElement
> +        only knows about ::before and ::after and not about valid pseudo-elements. We remove that method and create an
> +        equivalent in WebAnimationUtilities that knows about all public pseudo-elements.

Why not fix pseudoElementNameForEvents to handle all pseudo elements?

> Source/WebCore/animation/WebAnimationUtilities.cpp:182
> +String pseudoIdAsString(PseudoId pseudoId)
> +{

Yeah, I think this should stay on PseudoElement. If you were going to update the list, it's where you would look. Not inside WebAnimationUtilities.
Comment 5 Antoine Quint 2020-11-10 01:31:28 PST
Actually, we want to move away from PseudoElement which are only ever created for ::before and ::after pseudo-elements. We've moved to using the Styleable struct throughout the animation code so that we rely on PseudoElement as little as possible.

We only ever need to convert from a PseudoId to a String for animations, so I believe this is the right place.
Comment 6 EWS 2020-11-10 03:36:13 PST
Committed r269623: <https://trac.webkit.org/changeset/269623>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 413677 [details].