Bug 76131 – Text nodes should be merged when deleting a newline (causes wrong line wrapping behavior)

• Component: Text
• Status: NEW

[Alex Henrie]

Created attachment 122136 [details]
Strange text wrapping behavior.html

I've found some strange inconsistencies in WebKit's text wrapping behavior. See the attached file for details and examples. While this may not seem like a serious bug, it can cause problems for applications that depend on consistent and predictable text rendering. Also, WebKit's behavior most likely violates the CSS3 and UAX14 standards.

Let me know if you'd like any more information on this bug or if you know how it could be fixed.

See also:
http://www.w3.org/TR/css3-text/#wrapping
http://www.unicode.org/reports/tr14/tr14-17.html

See bug 17427.

[Alexey Proskuryakov]

Thanks Mitz! Ryosuke, it seems that the symptom could potentially be resolved by changes to editing code - is this something we'd like to track in this bug, or should it be marked duplicate?

[Ryosuke Niwa]

(In reply to comment #2)
> Thanks Mitz! Ryosuke, it seems that the symptom could potentially be resolved by changes to editing code - is this something we'd like to track in this bug, or should it be marked duplicate?

I think there's a bug in editing code that results in text nodes not being merged. When we deleted the line break, we should be merging the two text nodes.

[Joe Thomas]

Created attachment 123362 [details]
Working Patch

Found out that the issue lies in the asciiLineBreakTable in Source/WebCore/rendering/break_lines.cpp. In asciiLineBreakTable, if there is an opening punctuation {, [,( after a closing punctuation }, ], ), then it is considered that it can be breakable in between. And it is mentioned that this is added to make it compatible with Firefox 3.6 in the comments. 

Attaching a patch which solves the curly bracket issue '}{', which is mentioned in the test case. If the approach is right, I think we should do it for all combinations of {, [, (. 

Checked http://www.unicode.org/reports/tr14/tr14-17.html#ConfProperties, it only says that line break is prohibited before a closing punctuation and after an opening punctuation. Does not talk about whether we can break in between these opening and closing punctuation. Any other documentation that I can refer to ?

[Alex Henrie]

(In reply to comment #4)
> Checked http://www.unicode.org/reports/tr14/tr14-17.html#ConfProperties, it only says that line break is prohibited before a closing punctuation and after an opening punctuation. Does not talk about whether we can break in between these opening and closing punctuation. Any other documentation that I can refer to ?

Reading UAX14 more carefully, I noticed that the section "Line Breaking Algorithm" concludes with "Finally, join alphabetic letters and break everything else." So Firefox is incorrect and the correct behavior would be to always allow line breaks between {a}{b}. I checked and this is actually what Opera does already, as well as all the word processors I tested (AbiWord, LibreOffice, and Leafpad).

See http://www.unicode.org/reports/tr14/tr14-17.html#Algorithm

[Ryosuke Niwa]

(In reply to comment #4)
> Created an attachment (id=123362) [details]
> Working Patch

This is a patch for the bug 17427. This bug is about text nodes not being merged when deleting a newline.

[Joe Thomas]

(In reply to comment #6)
> (In reply to comment #4)
> > Created an attachment (id=123362) [details] [details]
> > Working Patch
> 
> This is a patch for the bug 17427. This bug is about text nodes not being merged when deleting a newline.

The patch was for the incorrect behavior mentioned as part of the attached test case Strange_text_wrapping_behavior.html. Title of the bug is a bit confusing for me with the test case attached. I will make the patch obsolete.

[Alexey Proskuryakov]

There are (at least) two ways to fix this specific test case, making wrapping consistent. This bug tracks one of them, bug 17427 tracks another. Both need to be done eventually.

It's also nice to figure out which behavior is correct, but this bug wasn't asking for that explicitly, just asking it to be consistent.

Hope this helps.

[Alex Henrie]

Created attachment 125912 [details]
Hack

This patch is a hack that merges the text nodes properly but messes up the caret position. This code is obviously not meant to be committed to WebKit, but I hope that it helps someone more familiar with WebKit than me to locate the problem and come up with a real solution.