Bug 111675 - When we set word-wrap: break-word and xml:space="preserve" to svg text element, the text is collapsed.
Summary: When we set word-wrap: break-word and xml:space="preserve" to svg text elemen...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-06 21:01 PST by Yuki Sekiguchi
Modified: 2013-03-08 06:35 PST (History)
8 users (show)

See Also:


Attachments
reproduced html content (325 bytes, text/html)
2013-03-06 21:01 PST, Yuki Sekiguchi
no flags Details
Patch (4.39 KB, patch)
2013-03-06 21:15 PST, Yuki Sekiguchi
no flags Details | Formatted Diff | Diff
Patch (4.77 KB, patch)
2013-03-07 20:31 PST, Yuki Sekiguchi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuki Sekiguchi 2013-03-06 21:01:31 PST
Created attachment 191909 [details]
reproduced html content

In the attached content, the text "abc" is SVG text and it have xml:space="preserve" attribute and is specified word-wrap: break-word.
The text is collapsed.

In the following spec, SVG don't perform automatic line break or word wrapping.
http://www.w3.org/TR/SVG/text.html#Introduction
> SVG performs no automatic line breaking or word wrapping.

Therefore, I think the collapsing is illegal.
Firefox don't collapse the text.
Comment 1 Yuki Sekiguchi 2013-03-06 21:15:48 PST
Created attachment 191912 [details]
Patch
Comment 2 Stephen Chenney 2013-03-07 07:12:39 PST
Comment on attachment 191912 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=191912&action=review

Mostly fine. I have one nit with the code and a request for greater testing.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2841
> +                breakWords = breakAll = false;

I believe that WebKit style dictates that this be two lines, with one assignment on each line.

> LayoutTests/svg/text/preserve-break-word-expected.html:6
> +<text x="100" y="100" style="word-wrap: break-word">abcdef</text>

Can you remove the style on the <text>? As I understand your patch, the SVG should appear the same as something with no word-wrap defined.

> LayoutTests/svg/text/preserve-break-word.html:6
> +<text x="100" y="100" style="word-wrap: break-word"><tspan xml:space="preserve">abc</tspan>def</text>

Could you please provide 3 <text> nodes? One with word-wrap: break word, one with xml:space="preserve" and one with both (the line you have already is the one with both).
Comment 3 Yuki Sekiguchi 2013-03-07 20:31:33 PST
Created attachment 192137 [details]
Patch
Comment 4 Yuki Sekiguchi 2013-03-07 20:42:42 PST
Hi Stephen,
Thank you for reviewing.

(In reply to comment #2)
> > Source/WebCore/rendering/RenderBlockLineLayout.cpp:2841
> > +                breakWords = breakAll = false;
> 
> I believe that WebKit style dictates that this be two lines, with one assignment on each line.

OK. I understand it.

> > LayoutTests/svg/text/preserve-break-word-expected.html:6
> > +<text x="100" y="100" style="word-wrap: break-word">abcdef</text>
> 
> Can you remove the style on the <text>? As I understand your patch, the SVG should appear the same as something with no word-wrap defined.

Sorry. You are right.
Fixed.

> > LayoutTests/svg/text/preserve-break-word.html:6
> > +<text x="100" y="100" style="word-wrap: break-word"><tspan xml:space="preserve">abc</tspan>def</text>
> 
> Could you please provide 3 <text> nodes? One with word-wrap: break word, one with xml:space="preserve" and one with both (the line you have already is the one with both).

Added test case.
Comment 5 Stephen Chenney 2013-03-08 06:28:06 PST
Looks good. R=me.
Comment 6 WebKit Review Bot 2013-03-08 06:35:04 PST
Comment on attachment 192137 [details]
Patch

Clearing flags on attachment: 192137

Committed r145215: <http://trac.webkit.org/changeset/145215>
Comment 7 WebKit Review Bot 2013-03-08 06:35:07 PST
All reviewed patches have been landed.  Closing bug.