Bug 12556

Summary: SVG text ignores xml:space attribute
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Initial patch
darin: review-
Updated patch sam: review+

Description Nikolas Zimmermann 2007-02-03 05:36:12 PST
SVG text has to handle xml:space="preserve".
And SVG text is never allowed to contain tabs.

Fixing that will fix some W3C-SVG-1.1 included tests.
Attaching patch.
Comment 1 Nikolas Zimmermann 2007-02-03 05:37:09 PST
Created attachment 12896 [details]
Initial patch

This contains some RenderStyle.h changes - I'd feel good if maybe Hyatt/Maciej can have a look here, too.
Comment 2 Darin Adler 2007-02-03 12:13:31 PST
Comment on attachment 12896 [details]
Initial patch

This new SVG_PRE value should not be in the SVG-specific files nor should it be ifdef'd.

How exactly is SVG_PRE different from PRE_WRAP?

+    // SVG text never contains tabs!

Are you sure this is the best way to do this? We specifically don't do this to change '\n' characters into ' ' characters.

Definitely should discuss these changes with Hyatt.

Given the number of different places changed, I want to see a separate test for each one. Basically if I take out any one of the SVG_PRE checks, a test should fail. We need to construct those tests.
Comment 3 Nikolas Zimmermann 2007-02-03 16:57:30 PST
(In reply to comment #2)
> (From update of attachment 12896 [details] [edit])
> This new SVG_PRE value should not be in the SVG-specific files nor should it be
> ifdef'd.
> 
> How exactly is SVG_PRE different from PRE_WRAP?
> 
> +    // SVG text never contains tabs!
> 
> Are you sure this is the best way to do this? We specifically don't do this to
> change '\n' characters into ' ' characters.

I had a chat with Mitz & Sam about these things and it turns out there is a much less intrusive possiblity to handle xml:space correctly, attaching new patch soon.

Niko
Comment 4 Nikolas Zimmermann 2007-02-03 16:59:49 PST
Created attachment 12906 [details]
Updated patch

Includes a new layout test testing all specified (SVG 1.1 spec) whitespace handling possibilities.
Comment 5 Sam Weinig 2007-02-03 17:28:09 PST
Comment on attachment 12906 [details]
Updated patch

With the one change we discussed in IRC, r=me.
Comment 6 Nikolas Zimmermann 2007-02-03 17:34:12 PST
Landed in r19390.