[webkit-reviews] review granted: [Bug 58294] continue (3rd) experiment with moving caret by word in visual order : [Attachment 89674] patch w/ layout test

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Xiaomei Ji <xji at chromium.org>'s
request for review:
Bug 58294: continue (3rd) experiment with moving caret by word in visual order
https://bugs.webkit.org/show_bug.cgi?id=58294

Attachment 89674: patch w/ layout test
https://bugs.webkit.org/attachment.cgi?id=89674&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=89674&action=review

(In reply to comment #12)
> (In reply to comment #10)
> > When block's direction is RTL, the word boundary are collected from left to
right visually. The function looks for left word boundary of the current word,
it actually looks for the right-most word boundary of a previous box (or the
right-most word boundary of the current box assuming that it is not the same as
the word itself).  visually last word boundary in RTL block is the right-most
word boundary we are looking for.
> 
> Ah, I finally understand. leftWordBoundary and rightWordBoundary are called
only when we're at the boundary of a box.  We should rename these two functions
to signify that fact or add a comment.

Please address this point.

> Source/WebCore/ChangeLog:49
> +	      based on the directionality of the box and block. These
computations do not consider the 

Nit: should probably say "These computations do not currently consider" to
signify the fact they should in the future.

> Source/WebCore/editing/visible_units.cpp:1375
> +    } while (!isLastWordBreakInBox);       

Why do we need to use do-while?  isLastWordBreakInBox is always false in the
first iteration.