[Webkit-unassigned] [Bug 61346] --webkit-visual-word: ctrl-arrow is not able to reach the boundary of line

[bugzilla-daemon at webkit.org]

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


Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #102185|review?                     |review-
               Flag|                            |




--- Comment #19 from Ryosuke Niwa <rniwa at webkit.org>  2011-07-27 17:19:36 PST ---
(From update of attachment 102185)
View in context: https://bugs.webkit.org/attachment.cgi?id=102185&action=review

> Source/WebCore/editing/visible_units.cpp:1188
> +static VisiblePosition leftmostPositionInRTLBoxInRTLBlock(const InlineBox* box)

You should probably add inline keyboard here.

> Source/WebCore/editing/visible_units.cpp:1218
> +        if (box->bidiLevel() == 1) {

I don't think we should be checking that the bidi level is 1.  If anything it should be checking that it's odd.  r- because of this.

> Source/WebCore/editing/visible_units.cpp:1257
> +    if (needToUseLogicalEndOfLine(box, blockDirection))
> +        return logicalEndOfLine(createPositionAvoidingIgnoredNode(box->renderer()->node(), box->caretMaxOffset()));
> +    
> +    return endOfBoxPosition(box, blockDirection);

I don't understand the difference between logicalEndOfLine and endOfBoxPosition.  We need to give them more descriptive name such that readers can make sense of this code without having to read the code in those two functions.

> Source/WebCore/editing/visible_units.cpp:1262
> +    return blockDirection == LTR ? !box->nextLeafChild() || box->nextLeafChild()->renderer()->isBR() : !box->prevLeafChild() || box->prevLeafChild()->renderer()->isBR();

Nit: I'd split this into two lines.

> Source/WebCore/editing/visible_units.cpp:1460
> +        endOfBlock.getInlineBoxAndOffset(boxOfEndOfBlock, offsetOfEndOfBlock);

positionIsInBox calls getInlineBoxAndOffset.  r- because of this.

> Source/WebCore/editing/visible_units.cpp:1485
> +        endOfBlock.getInlineBoxAndOffset(boxOfEndOfBlock, offsetOfEndOfBlock);

Ditto.

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