Summary: | REGRESSION (r15418): contenteditable div truncates rightmost Japanese character | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dan Wood <dwood> | ||||||||||
Component: | HTML Editing | Assignee: | Graham Dennis <Graham.Dennis> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Major | CC: | ap, Graham.Dennis, mitz | ||||||||||
Priority: | P1 | Keywords: | EasyFix, HasReduction, Regression | ||||||||||
Version: | 420+ | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.4 | ||||||||||||
Attachments: |
|
Description
Dan Wood
2006-09-13 11:10:42 PDT
Created attachment 10538 [details]
test case
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.
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). 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)
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? 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 on attachment 10609 [details]
patch 2
Make sure all other layout tests pass. r=me.
(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. Committed revision 16696. |