Bug 10842 – REGRESSION (r15418): contenteditable div truncates rightmost Japanese character

• Component: HTML Editing
• Status: RESOLVED FIXED

[Dan Wood]

When a div containing Japanese text is content-editable, the rightmost character does not display; it's truncated.

My test source text in Japanese (understanding the words it not needed; just look at the glyphs) is:

???????? ??????????????????????????????????Edgies?????US$9.99???????????????????????????????????

I've inserted hard returns in the text to see how the text wraps in the current released version of Safari/WebKit:

???????? ??
???????????
???????????
??????????
Edgies?????
US$9.99??????
???????????
???????????
???????

In TOT Webkit, if contentediting is not true, then the above is how it wraps -- that's correct.

However, if contentediting is true (focused or not), most of the rightmost characters (the ones near the right margin) are missing!  This is what you see:

???????? ?             missing: ?
??????????           missing:?
??????????           missing:?
??????????
Edgies?????
US$9.99?????              missing: ?
??????????           missing:?
??????????           missing:?
???????

[Dan Wood]

Created attachment 10538 [details]
test case

[Dan Wood]

Created attachment 10539 [details]
OK, it looks like I can't type Japanese into Bugzilla.  I've attached my report as a UTF16 text file.

[Alexey Proskuryakov]

Regression->P1

Oops, this is a regression from r15418 http://trac.webkit.org/projects/webkit/changeset/15418 (fix for bug 9670).

Looks like an easy fix: there are two places in RenderBlock::findNextLineBreak() that use the condition "o->style()->breakOnlyAfterWhiteSpace() && !midWordBreak": when setting charWidth and in the second if statement after that. You should add "currentCharacterIsWS && " in front of the condition (it is really only for checking whether to add a run of whitespace to the line even though it makes it too wide).

[Graham Dennis]

Created attachment 10598 [details]
patch

Patch using Mitz's suggesstion.

I also did a logic shuffle at Mitz's request to prevent re-testing the same expression twice in quick succession in two if statements (currentCharacterIsWS && o->style()->breakOnlyAfterWhiteSpace() && !midWordBreak)

[David Kilzer (:ddkilzer)]

Comment on attachment 10598 [details]
patch

>Index: LayoutTests/fast/text/line-breaks-after-white-space.html
>===================================================================
>Cannot display: file marked as a binary type.
>svn:mime-type = application/octet-stream
>
>Property changes on: LayoutTests/fast/text/line-breaks-after-white-space.html
>___________________________________________________________________
>Name: svn:mime-type
>   + application/octet-stream

The layout test should not have its svn mime type set to application/octet-stream, should it?  Did this happen automatically because of the unicode characters in the file?

[Graham Dennis]

Created attachment 10609 [details]
patch 2

Originally the layout test had Japanese characters in UTF16, but I ended up changing the patch to only use latin characters. Anyway, the mime type shouldn't have been application/octet-stream, and that's fixed now.

[Dave Hyatt]

Comment on attachment 10609 [details]
patch 2

Make sure all other layout tests pass.  r=me.

[Graham Dennis]

(In reply to comment #9)
> (From update of attachment 10609 [details] [edit])
> Make sure all other layout tests pass.  r=me.
> 

It doesn't break any layout tests.

[Alexey Proskuryakov]

Committed revision 16696.