Bug 67522

Summary: REGRESSION: Moving up doesn't work in some cases
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, cshu, enrica, eric, sullivan, tony, wcarss
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://plexode.com/eval3/#ht=%3Cdiv%20contenteditable%3Dtrue%3E%0A%3Cdiv%3Efoo%20bar%20foo%20bar%20skldjf%20klasdfj%20klasjdf%20kljas%20dklfja%20skdfj%20aksldjf%20kljsd%20fkljasd%20fkljas%20dfklj%20asdklfj%20klsdjfkljsd%20fkljsdfklj%20askldfj%20klasdjf%20laksdj%20sjhdfklasjdfkl%20ajsdklfj%20asdj%20flksjd%20klja%20sdfklj%20asdkljf%20lkadf%20lkj%20sdfjasdj%20flj%20asdfj%20a%20sdkfj%20j%20sdfjal%3Bdskfj%20%3Bj%20s%3Bdlfja%3Bsf%26nbsp%3B%3C%2Fdiv%3E%3Cdiv%3E%3Cbr%3E%3C%2Fdiv%3E%0A%3C%2Fdiv%3E&ohh=1&ohj=1&jt=&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1
Attachments:
Description Flags
fixes the bug eric: review+

Ryosuke Niwa
Reported 2011-09-02 14:09:23 PDT
In the attached url, the caret should appear on the second line, NOT on the first line. i.e. when moving upwards, WebKit not skips lines that are soft-wrapped.
Attachments
fixes the bug (4.04 KB, patch)
2011-09-12 15:02 PDT, Ryosuke Niwa 		eric: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2011-09-12 15:02:13 PDT
Created attachment 107089 [details] fixes the bug
Eric Seidel (no email)
Comment 2 2011-09-12 15:04:17 PDT
Comment on attachment 107089 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=107089&action=review > Source/WebCore/editing/visible_units.cpp:536 > + Position pos = n->hasTagName(brTag) ? positionBeforeNode(n) : createLegacyEditingPosition(n, caretMaxOffset(n)); Should this be a helper inline? Is this concept needed elsewhere?
Ryosuke Niwa
Comment 3 2011-09-12 15:07:33 PDT
Comment on attachment 107089 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=107089&action=review >> Source/WebCore/editing/visible_units.cpp:536 >> + Position pos = n->hasTagName(brTag) ? positionBeforeNode(n) : createLegacyEditingPosition(n, caretMaxOffset(n)); > > Should this be a helper inline? Is this concept needed elsewhere? Maybe. I think I need to clean up as a part of getting rid of createLegacyEditingPosition. This is a problem whenever we create positions using caretMinOffset/caretMaxOffset. There's a very interesting interactions between TextIterator and other iteration classes, visible_units, and VisiblePosition and caretMinOffset/caretMaxOffset so I've been postponing it for now.
Ryosuke Niwa
Comment 4 2011-09-12 16:31:03 PDT
Committed r94988: <http://trac.webkit.org/changeset/94988>
Ryosuke Niwa
Comment 5 2011-09-12 16:32:03 PDT
Thanks for the review! landing it now.
Note You need to log in before you can comment on or make changes to this bug.