[webkit-reviews] review denied: [Bug 61346] --webkit-visual-word: ctrl-arrow is not able to reach the boundary of line : [Attachment 102219] Revised fix (with updated test cases)

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Van Lam <vanlam at google.com>'s
request for review:
Bug 61346: --webkit-visual-word: ctrl-arrow is not able to reach the boundary
of line
https://bugs.webkit.org/show_bug.cgi?id=61346

Attachment 102219: Revised fix (with updated test cases)
https://bugs.webkit.org/attachment.cgi?id=102219&action=review

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


> Source/WebCore/editing/visible_units.cpp:1179
> +				      !box->prevLeafChild() ||
box->prevLeafChild()->renderer()->isBR();

Nit: We don't align second lines like this.  This line should be intended by 4
spaces in addition to the 4 spaces in front of "return".

>> Source/WebCore/editing/visible_units.cpp:1402
>> +	
> 
> the above 2 newly added code pieces are similar, maybe they could be combined
as one function? and logical of isBoxVisuallyLastInLine can be moved inside the
function as well.

I agree with Xiaomei here.  We should at least create a function for the code
inside the if.	r- because of this code duplication.