[webkit-reviews] review granted: [Bug 218741] [Web Animations] KeyframeEffect.pseudoElement does not return a valid string when targeting ::marker or ::first-letter : [Attachment 413677] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Antoine Quint <graouts at webkit.org>'s
request for review:
Bug 218741: [Web Animations] KeyframeEffect.pseudoElement does not return a
valid string when targeting ::marker or ::first-letter
https://bugs.webkit.org/show_bug.cgi?id=218741

Attachment 413677: Patch

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




--- Comment #4 from Dean Jackson <dino at apple.com> ---
Comment on attachment 413677
  --> https://bugs.webkit.org/attachment.cgi?id=413677
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.