Bug 59941 – REGRESSION(79985): Changes in fill-opacity should trigger repaint but don't

• Component: SVG
• Status: RESOLVED FIXED

[Nico Weber]

Created attachment 91934 [details]
repro

Repro:
1. Open attached file
2. Click circle

Expected: Circle opacity change is displayed immediately
Expected: Nothing seems to happen. After resizing the window, a repaint happens and things look correct.

Also see http://crbug.com/80880

[Dirk Schulze]

Regression introduced with bug 54800

[Rob Buis]

Hi,

I already have a fix for this so once I have a testcase I'll post a patch, expect somthing in the next 24h.
Cheers,

Rob.

[Rob Buis]

Created attachment 93235 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 93235 [details]
Patch

OK.  Do we already do the same for stroke?

[Eric Seidel (no email)]

Comment on attachment 93235 [details]
Patch

Why do we need to do this via a click?  Seems  round-about way to test this.  Why not just change the fill-opacity on load?

[Eric Seidel (no email)]

Comment on attachment 93235 [details]
Patch

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

> LayoutTests/svg/custom/fill-opacity-update.svg:5
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" 
> +  "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
> +<svg width="6cm" height="5cm" viewBox="0 0 600 500"
> +     xmlns="http://www.w3.org/2000/svg" version="1.1" onload="setTimeout('runTest()', 0)">

More junk here than needed.  Also, do we need a waitUntilDone since you're using a timeout?

[Eric Seidel (no email)]

Comment on attachment 93235 [details]
Patch

The fix is fine, but the test is bad.  Don't you want this to be a repaint test?

[Rob Buis]

Hi Eric,

(In reply to comment #7)
> (From update of attachment 93235 [details])
> The fix is fine, but the test is bad.  Don't you want this to be a repaint test?

Oops, when I woke up I saw r+ in my inbox so I went ahead with the commit. Will have a look at a better test now, and a separate stroke-opacity test is nice as well (though looking at the code that part was not broken).
Cheers,

Rob.

[Ademar Reis]

For reference, the (first) patch landed in r86331

[WebKit Review Bot]

http://trac.webkit.org/changeset/86331 might have broken Windows 7 Release (Tests)

[Rob Buis]

Created attachment 93291 [details]
Patch

[Rob Buis]

The improved test got landed in r86352, also an added test for stroke-opacity. I don't think we can avoid using notifyDone in this case. So I think this bug can be closed now.
Cheers,

Rob.

[Nico Weber]

Thanks!

[WebKit Review Bot]

http://trac.webkit.org/changeset/86352 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
svg/custom/fill-opacity-update.svg

[Ademar Reis]

Revision r86331 cherry-picked into qtwebkit-2.2 with commit 674fb8f <http://gitorious.org/webkit/qtwebkit/commit/674fb8f>
Revision r86352 cherry-picked into qtwebkit-2.2 with commit d2b42e8 <http://gitorious.org/webkit/qtwebkit/commit/d2b42e8>