Bug 17129

Summary: XML ProcessingInstructions do not support title= or alternate= attributes
Product: WebKit Reporter: Oliver Hunt <oliver>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, zimmermann
Priority: P2 Keywords: NeedsReduction
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://treebuilder.de/default.asp?file=388729.xml
Attachments:
Description Flags
Copy of test svg
none
First attempt eric: review+

Description Oliver Hunt 2008-01-31 17:40:32 PST
The referenced URL allows you to choose between multiple style sheets for the svg doc, however the first rendering seems to have an incorrect style (cf. firefox3 rendering which seems sane).  Furthermore, clicking the little coloured squares should trigger a style change but results in the svg going black.
Comment 1 Oliver Hunt 2008-02-05 03:57:34 PST
Created attachment 18934 [details]
Copy of test svg

Copy of the svg that trigger this.

From looking at this i'm guessing that the bug is that we arbitrarily take the last style sheet reference, regardless of the alternate attribute.  When then toss all the other style sheets so subsequent attempts to switch to those style sheets fails.
Comment 2 Rob Buis 2008-04-05 08:41:40 PDT
Created attachment 20353 [details]
First attempt

It turned out this is not really a svg bug, but an omission in the xml-stylesheet handling. This patch fixes that.
Cheers,

Rob.
Comment 3 Rob Buis 2008-04-05 11:15:39 PDT
I forgot to mention that FireFox passes the same tests, so it must do the pseudo attribute processing too.
Cheers,

Rob.
Comment 4 Eric Seidel (no email) 2008-04-05 13:23:24 PDT
Comment on attachment 20353 [details]
First attempt

Ok, this patch looks good.  Unfortunately the method names are confusing (not your fault).
Comment 5 Eric Seidel (no email) 2008-04-05 13:26:24 PDT
Comment on attachment 20353 [details]
First attempt

Oh, one style comment.

single line ifs, the contents should be on the second line:
if (foo) return;
should be:
if (foo)
    return;

Also, if you're going to use:
if (foo != true) instead of the normal if (!foo) for that case, then you should use:
if (foo !== true) to actually check the type returned.  Otherwise:
if (foo) or if (!foo) is doing the same thing and should be used instead.
Comment 6 Rob Buis 2008-04-05 13:47:27 PDT
Hi Eric,

(In reply to comment #5)
> (From update of attachment 20353 [details] [edit])
> Oh, one style comment.
> 
> single line ifs, the contents should be on the second line:
> if (foo) return;
> should be:
> if (foo)
>     return;
> 
> Also, if you're going to use:
> if (foo != true) instead of the normal if (!foo) for that case, then you should
> use:
> if (foo !== true) to actually check the type returned.  Otherwise:
> if (foo) or if (!foo) is doing the same thing and should be used instead.

Oops, I saw this only after my commit. I assume that was code from the testcase(s). Let me know if it needs fixing in a new commit.
Cheers,

Rob.

Comment 7 Eric Seidel (no email) 2008-04-05 14:48:28 PDT
It's OK.  I'll just be more picky about style next time. ;)