Bug 10842 - REGRESSION (r15418): contenteditable div truncates rightmost Japanese character
Summary: REGRESSION (r15418): contenteditable div truncates rightmost Japanese character
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Major
Assignee: Graham Dennis
Keywords: EasyFix, HasReduction, Regression
Depends on:
Reported: 2006-09-13 11:10 PDT by Dan Wood
Modified: 2006-10-01 02:09 PDT (History)
3 users (show)

See Also:

test case (1.13 KB, text/html)
2006-09-13 11:11 PDT, Dan Wood
no flags Details
OK, it looks like I can't type Japanese into Bugzilla. I've attached my report as a UTF16 text file. (1.99 KB, text/plain)
2006-09-13 11:14 PDT, Dan Wood
no flags Details
patch (31.96 KB, patch)
2006-09-17 02:04 PDT, Graham Dennis
no flags Details | Formatted Diff | Diff
patch 2 (31.05 KB, patch)
2006-09-17 15:24 PDT, Graham Dennis
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dan Wood 2006-09-13 11:10:42 PDT
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:

???????? ??

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:?
US$9.99?????              missing: ?
??????????           missing:?
??????????           missing:?
Comment 1 Dan Wood 2006-09-13 11:11:45 PDT
Created attachment 10538 [details]
test case
Comment 2 Dan Wood 2006-09-13 11:14:43 PDT
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.
Comment 3 Alexey Proskuryakov 2006-09-13 21:30:39 PDT
Comment 4 mitz 2006-09-14 01:00:27 PDT
Oops, this is a regression from r15418 http://trac.webkit.org/projects/webkit/changeset/15418 (fix for bug 9670).
Comment 5 mitz 2006-09-14 23:35:55 PDT
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).
Comment 6 Graham Dennis 2006-09-17 02:04:35 PDT
Created attachment 10598 [details]

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)
Comment 7 David Kilzer (:ddkilzer) 2006-09-17 08:29:03 PDT
Comment on attachment 10598 [details]

>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?
Comment 8 Graham Dennis 2006-09-17 15:24:28 PDT
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.
Comment 9 Dave Hyatt 2006-09-18 23:17:14 PDT
Comment on attachment 10609 [details]
patch 2

Make sure all other layout tests pass.  r=me.
Comment 10 Graham Dennis 2006-09-24 06:11:50 PDT
(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.
Comment 11 Alexey Proskuryakov 2006-10-01 02:09:26 PDT
Committed revision 16696.