Bug 67522 - REGRESSION: Moving up doesn't work in some cases
Summary: REGRESSION: Moving up doesn't work in some cases
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Ryosuke Niwa
URL: http://plexode.com/eval3/#ht=%3Cdiv%2...
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-02 14:09 PDT by Ryosuke Niwa
Modified: 2011-09-12 16:32 PDT (History)
7 users (show)

See Also:


Attachments
fixes the bug (4.04 KB, patch)
2011-09-12 15:02 PDT, Ryosuke Niwa
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 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.
Comment 1 Ryosuke Niwa 2011-09-12 15:02:13 PDT
Created attachment 107089 [details]
fixes the bug
Comment 2 Eric Seidel (no email) 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?
Comment 3 Ryosuke Niwa 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.
Comment 4 Ryosuke Niwa 2011-09-12 16:31:03 PDT
Committed r94988: <http://trac.webkit.org/changeset/94988>
Comment 5 Ryosuke Niwa 2011-09-12 16:32:03 PDT
Thanks for the review! landing it now.