Bug 269481 – Animations do not work unless y1 / y2 are specified

• Status: REOPENED
• Component: SVG

[Jamie Burchell]

The follow SVG should animate but will not unless y1="0" (the default) is added to the first linearGradient:

<svg xmlns="http://www.w3.org/2000/svg" width="140" height="140">
<linearGradient id="reuseme" gradientUnits="userSpaceOnUse" spreadMethod="reflect">
    <stop offset="0" stop-color="red"/>
    <stop offset="1" stop-color="black"/>
    <animate attributeName="y1" dur="2s" repeatCount="indefinite" values="0%;150%;0%"/>
</linearGradient>
<linearGradient id="a" x1="22.677" x2="132.625" href="#reuseme"/>
<rect fill="url(#a)" x="10" y="10" width="130" height="130"/>
</svg>

[Jamie Burchell]

Example SVGs and discovery of this bug here https://stackoverflow.com/a/78001315/4810214

[Ahmad Saleem]

It is reproducible on WebKit ToT (274787@main) and also in LBSE as well.

Adding BrowserCompat since it is working fine in both Chrome Canary 123 and Firefox Nightly 124.

Adding `Antoine` to get input, if it is something on Animation side and not SVG but as mentioned in StackOverflow and in Comment 0 - if added `y1="0"` then it works. So most probably could be an issue in SVG area.

[Ahmad Saleem]

Note - in commit: 257032@main, we fixed default values for linearGradient.

Only thing I have tried so far is `LinearGradientAttributes.h`, I explicitly also updated this:

struct LinearGradientAttributes : GradientAttributes {
    LinearGradientAttributes()
        : m_x1(SVGLengthMode::Width, "0%"_s)
        , m_y1(SVGLengthMode::Height, "0%"_s)
        , m_x2(SVGLengthMode::Width, "100%"_s)
        , m_y2(SVGLengthMode::Height, "0%"_s)
        , m_x1Set(false)
        , m_y1Set(false)
        , m_x2Set(false)
        , m_y2Set(false)
    {
    }

but it didn't fix the test case.

[Jamie Burchell]

A simpler, minimally reproducible example SVG:

<svg xmlns="http://www.w3.org/2000/svg" width="140" height="140">
<linearGradient id="a">
    <stop offset="0" stop-color="red"/>
    <stop offset="1" stop-color="black"/>
    <animate attributeName="y1" dur="2s" repeatCount="indefinite" values="0%;150%;0%"/>
</linearGradient>
<rect fill="url(#a)" x="10" y="10" width="130" height="130"/>
</svg>

[Ahmad Saleem]

(In reply to Jamie Burchell from comment #4)
> A simpler, minimally reproducible example SVG:
> 
> <svg xmlns="http://www.w3.org/2000/svg" width="140" height="140">
> <linearGradient id="a">
>     <stop offset="0" stop-color="red"/>
>     <stop offset="1" stop-color="black"/>
>     <animate attributeName="y1" dur="2s" repeatCount="indefinite"
> values="0%;150%;0%"/>
> </linearGradient>
> <rect fill="url(#a)" x="10" y="10" width="130" height="130"/>
> </svg>

Good news! I manage to fix this in local build and it is working but StackOverflow example is not working.

Will do PR on weekend to get this over the line and get this sorted.

[Ahmad Saleem]

Created attachment 469917 [details]
Local Build - Screen Recording - Working Test Case

[Jamie Burchell]

> Good news! I manage to fix this in local build and it is working but
> StackOverflow example is not working.

You should not expect the very first SVG example on that SO question to work on any platform as that was an error on my part. The other two examples should work.

[Antoine Quint]

Ahmad, as you worked out, this is most likely an SVG issue. Thanks for looking into this!

[Ahmad Saleem]

PR - https://github.com/WebKit/WebKit/pull/24683

Might need to mark few tests as failing expectations but in LBSE - it passes all of them, so big progress.

All attached test in JSFiddle: https://jsfiddle.net/gz7keLdc/2/show

[Radar WebKit Bug Importer]

<rdar://problem/123457088>

[EWS]

Committed 310257@main (a6a8ca738974): <https://commits.webkit.org/310257@main>

Reviewed commits have been landed. Closing PR #24683 and removing active labels.

[Ahmad Saleem]

*** Bug 88195 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Re-opened since this is blocked by bug 313018

[Brent Fulgham]

This change is present in Safari Technology Preview 242. Please install that revision or newer to confirm the fix.

Release notes: https://webkit.org/blog/17934/release-notes-for-safari-technology-preview-242/

[Jamie Burchell]

Sorry, I don't have access to Safari to test this.

[Brent Fulgham]

This change is present in Safari Technology Preview 241. Please install that revision or newer to confirm the fix.

Release notes: https://webkit.org/blog/17917/release-notes-for-safari-technology-preview-241/

[Brent Fulgham]

(In reply to Jamie Burchell from comment #15)
> Sorry, I don't have access to Safari to test this.

Did you notice this in a GTK or WPE system? I'm trying to get our build system to update Bugzilla when we ship fixes so that people can confirm for themselves that a change shipped (and when). Often these get left in limbo. But I don't have visibility into the Linux-based ports release schedule.

[Jamie Burchell]

It was noticed on Chrome and Safari on iOS