|Summary:||REGRESSION(79985): Changes in fill-opacity should trigger repaint but don't|
|Product:||WebKit||Reporter:||Nico Weber <thakis>|
|Component:||SVG||Assignee:||Rob Buis <rwlbuis>|
|Severity:||Normal||CC:||abarth, ademar, eric, krit, rwlbuis, thakis, webkit.review.bot, zimmermann|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
|Bug Depends on:||54800|
Description Nico Weber 2011-05-02 10:02:26 PDT
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
Comment 2 Rob Buis 2011-05-11 16:06:17 PDT
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.
Comment 4 Eric Seidel (no email) 2011-05-11 20:29:58 PDT
Comment on attachment 93235 [details] Patch OK. Do we already do the same for stroke?
Comment 5 Eric Seidel (no email) 2011-05-11 20:30:46 PDT
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?
Comment 6 Eric Seidel (no email) 2011-05-11 20:31:21 PDT
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?
Comment 7 Eric Seidel (no email) 2011-05-11 20:31:51 PDT
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?
Comment 8 Rob Buis 2011-05-12 06:37:52 PDT
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.
Comment 9 Ademar Reis 2011-05-12 07:25:24 PDT
For reference, the (first) patch landed in r86331
Comment 10 WebKit Review Bot 2011-05-12 08:45:44 PDT
http://trac.webkit.org/changeset/86331 might have broken Windows 7 Release (Tests)
Comment 12 Rob Buis 2011-05-12 10:02:39 PDT
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.
Comment 13 Nico Weber 2011-05-12 10:07:36 PDT
Comment 14 WebKit Review Bot 2011-05-12 11:10:26 PDT
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
Comment 15 Ademar Reis 2011-05-13 09:28:53 PDT
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>