[Webkit-unassigned] [Bug 57336] experiment with moving caret by word in visual order

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=57336


Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #87696|1                           |0
        is obsolete|                            |




--- Comment #14 from Ryosuke Niwa <rniwa at webkit.org>  2011-03-31 04:54:21 PST ---
(From update of attachment 87696)
View in context: https://bugs.webkit.org/attachment.cgi?id=87696&action=review

> Source/WebCore/editing/visible_units.cpp:1169
> +    // We need to take care of the word break is at the correct place (before space or after space).
> +    // Follow Firefox's convention in Windows, 
> +    // In LTR block, word break visually moves cursor to the left boundary of words,
> +    // In RTL block, word break visually moves cursor to the right boundary of words.
> +    //
> +    // Since we are using previousWordPosition() and nextWordPosition() to get the word boundary.
> +    // For LTR text, previousWordPosition() returns the word break at the left boundary of a word, 
> +    //               nextWordPosition() returns the word break at the right boundary of a word. 
> +    // For RTL text, previousWordPosition() returns the word break at the right boundary of a word,
> +    //               nextWordPosition() returns the word break at the left boundary of a word.
> +    // 
> +    // So, when the box's directionality is the same as the block's directionality,
> +    // we should use previousWordPosition() for LTR text or RTL text.
> +    // If continously collecting word breaks, for LTR box, the word breaks are collected from right to left.
> +    // for RTL box, word breaks are collected from left to right. 

This comment is too long.  Revise it something like:

// In a LTR block, the word break should be on the left boundary of a word.
// In a RTL block, the word break should be on the right boundary of a word.
// Because nextWordPosition() returns the word break on the right boundary of the word for LTR text,
// we need to use previousWordPosition() to traverse words within the inline boxes from right to left
// to find the previous word break (i.e. the first word break on the left). The same applies to RTL text.

> Source/WebCore/editing/visible_units.cpp:1174
> +    // FIXME: handle multi-spaces. 
> +    // In the case of multi-spaces, word break among the collapsed spaces might not added.
> +    // For example, "<div>abc    def hij    opq rst</div>", no word break is added between "hij" and "opq".

I can't make sense of this comment.  Please get help from Eric on this :)

> Source/WebCore/editing/visible_units.cpp:1219
> +            wordBreak = findWordBoundaryInBoxInBlockDirection(adjacentBox, adjacentBox == box ? offset : invalidOffset, blockDirection);

findWordBoundaryInBoxInBlockDirection isn't the right name for this function.  If we're in LTR context, and looking for a word boundary on the left, that means we're moving to the opposite direction.  Maybe should just call this function previousWordBreakInBox instead.

> Source/WebCore/editing/visible_units.cpp:1245
> +    // Follow Firefox's convention in Windows, 
> +    // In LTR block, word break visually moves cursor to the left boundary of words,
> +    // In RTL block, word break visually moves cursor to the right boundary of words.

This comment is irrelevant to the code below so please remove.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.