Bug 15343

Summary:

Many SVG tests "fail" due to changed text color dumping (after feature brach rebase)

Product:

WebKit

Reporter:

Eric Seidel (no email) <eric>

Component:

SVG

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

oliver, zimmermann

Priority:

Version:

523.x (Safari 3)

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
attempted (sorta) fix	none

Eric Seidel (no email)

Reported 2007-10-02 08:33:05 PDT

Many SVG tests "fail" due to changed text color dumping (after feature brach rebase) I believe the previous dumping method was not really determinant. Or at least it wasn't dumping what it was trying to dump. First of all, color() is not really the normal way to change text color in SVG. Second, it seems that sometimes that was the fill color, and sometimes the stroke. I believe color() was previously being modified by the painting code, but is no longer. I'm not sure why that would change with the rebase (perhaps all the tests got updated?) I have attached one attempted fix, but it doesn't really make the situation any better or worse. It also seems to still miss some colors that it should be dumping (I'm unsure why).

Attachments
attempted (sorta) fix (2.49 KB, patch) 2007-10-02 08:33 PDT, Eric Seidel (no email)	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Eric Seidel (no email)

Comment 1 2007-10-02 08:33:32 PDT

Created attachment 16504 [details] attempted (sorta) fix

Nikolas Zimmermann

Comment 2 2007-10-02 10:07:12 PDT

Yes, I updated all tests to not dump text colors at all in feature-branch. It's definately better than dumping invalid colors (as previously, as you said, the paint servers mutated color() when painting text - I changed that). I haven't checked the attached patch for correctness, as I'm really busy learning atm. Next week I can comment :-) Greetings, Niko

Oliver Hunt

Comment 3 2007-10-02 11:24:46 PDT

How many failures are left are this change?

Eric Seidel (no email)

Comment 4 2007-10-03 09:24:46 PDT

I suggest we just remove the "color()" call, or apply my patch, and rebase. My patch should probably be changed to label the colors its dumping. Such as "fillColor=" instead of "color=" How many tests still failed after this? Lots. A different subset of the SVG tests.

Eric Seidel (no email)

Comment 5 2007-10-03 09:33:19 PDT

I should note, that if we're going to apply this patch, the "else if"s should just turn into ifs. And the color dumps should possibly be re-ordered. I'm not sure that dumping solid colors for text is very useful though (especially since this patch doesn't seem to catch all cases anyway.

Nikolas Zimmermann

Comment 6 2007-10-06 17:25:07 PDT

Fixed in fb svn.

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