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

Tim Nguyen (:ntim)
Reported 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
Attachments
Patch (32.42 KB, patch)
2021-10-19 06:37 PDT, Antoine Quint 		no flags
DetailsFormatted DiffDiff
Patch for landing (32.90 KB, patch)
2021-10-19 09:17 PDT, Antoine Quint 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2021-10-18 12:45:57 PDT
<rdar://problem/84383279>
Tim Nguyen (:ntim)
Comment 2 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
Antoine Quint
Comment 3 2021-10-19 06:37:01 PDT
Created attachment 441718 [details] Patch
Antti Koivisto
Comment 4 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.
Antoine Quint
Comment 5 2021-10-19 09:17:06 PDT
Created attachment 441737 [details] Patch for landing
EWS
Comment 6 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].
Simon Fraser (smfr)
Comment 7 2021-10-22 11:44:02 PDT
The test is flakey: bug 232040.
Simon Fraser (smfr)
Comment 8 2021-10-22 14:02:06 PDT
The testcase shows an opacity animation triggering paints on the ::marker, which seems wrong.
Note You need to log in before you can comment on or make changes to this bug.