Bug 231906

Summary: REGRESSION(r284313): ::marker accelerated animations are broken
Product: WebKit Reporter: Tim Nguyen (:ntim) <ntim>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, graouts, graouts, koivisto, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=232040
Bug Depends on:    
Bug Blocks: 230008    
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Tim Nguyen (:ntim) 2021-10-18 12:45:46 PDT
<style>
@keyframes animate-marker {
  from {
    color: red;
    opacity: 1;
  }
  50% {
    color: green;
    opacity: 0;
  }
  to {
    color: blue;
    opacity: 1;
  }
}

::marker {
  animation: animate-marker 2s infinite; 
}
</style>

<li>Hello</li>

---

In the testcase, the marker disappears completely. Per spec, opacity should not apply at all, so the marker should be visible. But even if we did allow applying opacity, the opacity should animate properly.

regression of the `Styleable::fromRenderer` changes in bug 230008 for ::marker
Comment 1 Radar WebKit Bug Importer 2021-10-18 12:45:57 PDT
<rdar://problem/84383279>
Comment 2 Tim Nguyen (:ntim) 2021-10-18 12:47:28 PDT
This is the spec that says opacity should not apply on ::marker: https://www.w3.org/TR/css-lists-3/#marker-properties
Comment 3 Antoine Quint 2021-10-19 06:37:01 PDT
Created attachment 441718 [details]
Patch
Comment 4 Antti Koivisto 2021-10-19 08:38:51 PDT
Comment on attachment 441718 [details]
Patch

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

> Source/WebCore/style/PropertyAllowlist.cpp:41
> +// https://www.w3.org/TR/css-pseudo-4/#marker-pseudo (Editor's Draft, 25 July 2017)
> +// FIXME: this is outdated, see https://bugs.webkit.org/show_bug.cgi?id=218791.
> +bool isValidMarkerStyleProperty(CSSPropertyID id)

It sounded from spec reading in slack that this allow list is a wrong approach. All properties should be visible in the computed style and the rendering code should be ignoring whatever is disallowed.

This refactoring ends up being bit useless if we just delete all code here soon but I suppose it doesn't hurt.
Comment 5 Antoine Quint 2021-10-19 09:17:06 PDT
Created attachment 441737 [details]
Patch for landing
Comment 6 EWS 2021-10-19 10:18:13 PDT
Committed r284466 (243226@main): <https://commits.webkit.org/243226@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 441737 [details].
Comment 7 Simon Fraser (smfr) 2021-10-22 11:44:02 PDT
The test is flakey: bug 232040.
Comment 8 Simon Fraser (smfr) 2021-10-22 14:02:06 PDT
The testcase shows an opacity animation triggering paints on the ::marker, which seems wrong.