Bug 12569

Summary:

WebKit does not handle missing filter elements correctly

Product:

WebKit

Reporter:

Eric Seidel (no email) <eric>

Component:

SVG

Assignee:

Rob Buis <rwlbuis>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

adamk, dglazkov, jeffschiller, krit, rwlbuis, webkit.review.bot

Priority:

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

URL:

http://www.w3.org/Graphics/SVG/Test/20061213/htmlEmbedHarness/full-filters-felem-01-b.html

Bug Depends on:

5527

Bug Blocks:

68469, 26389

Attachments:

Description	Flags
Patch	eric: review+, webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-02	none

Eric Seidel (no email)

Reported 2007-02-04 03:24:43 PST

WebKit does not handle missing filter elements correctly This should be pretty easy to fix.

Attachments
Patch (30.33 KB, patch) 2011-05-25 09:19 PDT, Rob Buis	eric: review+ webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ec2-cr-linux-02 (1.25 MB, application/zip) 2011-05-25 11:43 PDT, WebKit Review Bot	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Eric Seidel (no email)

Comment 1 2007-02-06 07:51:25 PST

the top-right one is easy to fix. it means adding something like this to all the renderers: if (!filterId.isNull() && !filter) return;

Rob Buis

Comment 2 2011-05-25 09:19:13 PDT

Created attachment 94791 [details] Patch

Eric Seidel (no email)

Comment 3 2011-05-25 09:25:09 PDT

Comment on attachment 94791 [details] Patch So do We pass the test now? I see a lot of red, but I assume this is an improvement? Do we need another big to fix the rest? Could we file a bug with the w3c to fix their confusing test

Eric Seidel (no email)

Comment 4 2011-05-25 09:26:03 PDT

Eric Seidel (no email)

Comment 5 2011-05-25 09:49:14 PDT

Comment on attachment 94791 [details] Patch OK. Do we have more to fix here? Do we need to file another bug? I see a lot of red here. Is the test just confusing? Do we need to file a big with the w3c to have them unconfuse it?

Dirk Schulze

Comment 6 2011-05-25 10:10:24 PDT

Comment on attachment 94791 [details] Patch I'd like to see a test where we add the non-existant filter after the rendering, something like a repaint drt test. Also can you add the test from SVG1.1se test suite?

Rob Buis

Comment 7 2011-05-25 10:40:44 PDT

Hi Eric, (In reply to comment #5) > (From update of attachment 94791 [details]) > OK. Do we have more to fix here? Do we need to file another bug? I see a lot of red here. Is the test just confusing? Do we need to file a big with the w3c to have them unconfuse it? The SVG 2nd edition test seems a lot better: http://dev.w3.org/SVG/profiles/1.1F2/test/svg/filters-felem-01-b.svg I'll be sure to include this in the patch. Cheers, Rob.

Rob Buis

Comment 8 2011-05-25 11:08:41 PDT

To be clear, we do pass both 1.1 and 1.1 SE tests with my patch. Only the top-right circle was wrong, and the patch fixes it. Cheers, Rob. (In reply to comment #7) > Hi Eric, > > (In reply to comment #5) > > (From update of attachment 94791 [details] [details]) > > OK. Do we have more to fix here? Do we need to file another bug? I see a lot of red here. Is the test just confusing? Do we need to file a big with the w3c to have them unconfuse it? > > The SVG 2nd edition test seems a lot better: > > http://dev.w3.org/SVG/profiles/1.1F2/test/svg/filters-felem-01-b.svg > > I'll be sure to include this in the patch. > Cheers, > > Rob.

WebKit Review Bot

Comment 9 2011-05-25 11:43:13 PDT

Comment on attachment 94791 [details] Patch Attachment 94791 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8734497 New failing tests: inspector/profiler/cpu-profiler-profiling.html

WebKit Review Bot

Comment 10 2011-05-25 11:43:18 PDT

Created attachment 94819 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

Rob Buis

Comment 11 2011-05-25 12:02:06 PDT

Committed r87310: <http://trac.webkit.org/changeset/87310>

Adam Klein

Comment 12 2011-05-25 15:22:27 PDT

r87310 seems to have caused regressions on the Chromium/WebKit bots: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=svg%2FW3C-SVG-1.1-SE%2Ffilters-felem-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Ffilters-felem-01-b.svg&group=%40ToT%20-%20chromium.org For the new test, it looks like we just need to add baselines. But the old test regressed, and I can't tell for sure whether this is an improvement just by looking. Please take a look.

Adam Klein

Comment 13 2011-05-25 15:28:01 PDT

(In reply to comment #12) > r87310 seems to have caused regressions on the Chromium/WebKit bots: > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=svg%2FW3C-SVG-1.1-SE%2Ffilters-felem-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Ffilters-felem-01-b.svg&group=%40ToT%20-%20chromium.org > > For the new test, it looks like we just need to add baselines. But the old test regressed, and I can't tell for sure whether this is an improvement just by looking. Please take a look. Actually, looking more closely at the diffs, it looks like the Chromium results just need to be rebaselined all around.

Rob Buis

Comment 14 2011-05-25 15:29:58 PDT

(In reply to comment #13) > (In reply to comment #12) > > r87310 seems to have caused regressions on the Chromium/WebKit bots: > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=svg%2FW3C-SVG-1.1-SE%2Ffilters-felem-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Ffilters-felem-01-b.svg&group=%40ToT%20-%20chromium.org > > > > For the new test, it looks like we just need to add baselines. But the old test regressed, and I can't tell for sure whether this is an improvement just by looking. Please take a look. > > Actually, looking more closely at the diffs, it looks like the Chromium results just need to be rebaselined all around. Hi Adam, Agreed. It is an improvement btw and matches how the w3c want it to look. Cheers, Rob.

Adam Klein

Comment 15 2011-05-25 16:41:18 PDT

(In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #12) > > > r87310 seems to have caused regressions on the Chromium/WebKit bots: > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=svg%2FW3C-SVG-1.1-SE%2Ffilters-felem-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Ffilters-felem-01-b.svg&group=%40ToT%20-%20chromium.org > > > > > > For the new test, it looks like we just need to add baselines. But the old test regressed, and I can't tell for sure whether this is an improvement just by looking. Please take a look. > > > > Actually, looking more closely at the diffs, it looks like the Chromium results just need to be rebaselined all around. > > Hi Adam, > > Agreed. It is an improvement btw and matches how the w3c want it to look. > Cheers, > > Rob. Thanks. Rebaselines submitted in http://trac.webkit.org/changeset/87334.

Note You need to log in before you can comment on or make changes to this bug.