Bug 236491 - SVG SMIL restart="never" does not behave correctly
Summary: SVG SMIL restart="never" does not behave correctly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: Safari 15
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-11 01:06 PST by kari.pihkala
Modified: 2022-02-17 04:54 PST (History)
14 users (show)

See Also:

Attachments
SVG SMIL restart.svg test case (1.09 KB, image/svg+xml)
2022-02-11 01:06 PST, kari.pihkala 		no flags Details
Patch (5.04 KB, patch)
2022-02-14 04:34 PST, Antoine Quint 		koivisto: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (5.29 KB, patch)
2022-02-16 02:20 PST, Antoine Quint 		no flags Details | Formatted Diff | Diff
Patch for landing (5.32 KB, patch)
2022-02-16 09:39 PST, Antoine Quint 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description kari.pihkala 2022-02-11 01:06:04 PST 
Created attachment 451655 [details]
SVG SMIL restart.svg test case

The attached restart.svg test case shows three rectangles with SMIL animations and the restart attribute set to different values: "never", "always" and "whenNotActive".

The green rectangles (restart="always" and restart="whenNotActive") behave correctly. However, the blue rectangle with restart="never" does not behave correctly on Safari.

Steps to reproduce:

1. Open restart.svg
2. Click the blue rectangle.
3. Wait until the blue rectangle has moved up for a second and jumps back to its original position.
4. Click the blue rectangle again. The blue rectangle should not move again, but Safari moves it again. Safari behavior for restart="never" seems to match restart="whenNotActive".

Expected result: The blue rectangle is moved only once for the first click. Subsequent clicks don't animate the blue rectangle. Firefox and Chrome show the expected result.

Tested on Safari Version 15.3 (16612.4.9.1.7, 16612) and Safari Technology Preview Release 139 (Safari 15.4, WebKit 16613.1.14.41.2).

Here's the relevant specification: https://svgwg.org/specs/animations/#RestartAttribute
Comment 1 Antoine Quint 2022-02-14 02:44:18 PST 
It looks like "never" and "whenNotActive" behave the same.
Comment 2 Antoine Quint 2022-02-14 04:31:21 PST 
The "restart" value is not checked when we compute a new interval in SVGSMILElement::beginListChanged().
Comment 3 Antoine Quint 2022-02-14 04:34:57 PST 
Created attachment 451888 [details]
Patch
Comment 4 Antti Koivisto 2022-02-14 06:51:33 PST 
Comment on attachment 451888 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Test: svg/animations/restart-never-and-begin-click.html

WPT?
Comment 5 Antoine Quint 2022-02-16 02:20:09 PST 
Created attachment 452164 [details]
Patch for landing
Comment 6 Antoine Quint 2022-02-16 02:21:09 PST 
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/32856
Comment 7 EWS Watchlist 2022-02-16 02:22:17 PST 
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 8 kari.pihkala 2022-02-16 04:35:18 PST 
Thank you for fixing this so fast!

I noticed that restart-never-and-begin-click.html in the Patch (attachment 451888 [details]) says: "Setting restart='always' prevents animation with begin='click' from restarting". Shouldn't it say "Setting restart='never' "?
Comment 9 Antoine Quint 2022-02-16 05:28:09 PST 
(In reply to kari.pihkala from comment #8)
> Thank you for fixing this so fast!

You're welcome. Thanks for filing the bug!

> I noticed that restart-never-and-begin-click.html in the Patch (attachment
> 451888 [details]) says: "Setting restart='always' prevents animation with
> begin='click' from restarting". Shouldn't it say "Setting restart='never' "?

Oh, indeed! I will fix this when I finally commit the change.
Comment 10 Antoine Quint 2022-02-16 09:39:15 PST 
Created attachment 452202 [details]
Patch for landing
Comment 11 Antoine Quint 2022-02-17 04:53:45 PST 
Committed r290010 (247396@trunk): <https://commits.webkit.org/247396@trunk>
Comment 12 Radar WebKit Bug Importer 2022-02-17 04:54:18 PST 
<rdar://problem/89080968>