[Webkit-unassigned] [Bug 65277] Make functions to find word boundaries more flexible

[bugzilla-daemon at webkit.org]

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


Ryosuke Niwa <rniwa at webkit.org> changed:

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




--- Comment #48 from Ryosuke Niwa <rniwa at webkit.org>  2011-08-24 20:03:10 PST ---
(From update of attachment 104591)
View in context: https://bugs.webkit.org/attachment.cgi?id=104591&action=review

> Source/WebCore/editing/visible_units.cpp:1591
> +static VisiblePosition nextWordBoundaryBeforeWord(VisiblePosition startingPosition, InlineBox* startingBox, int startingOffset)

This function's interface is quite confusing.  Why does this function take BOTH startingPosition and startingBox & startingOffset.  I'd rather make an extra call to getInlineBoxAndOffset than requiring callers to pass in the correct arguments.  It appears that this function wants to take RenderedPosition as the argument after all.

> Source/WebCore/editing/visible_units.cpp:1593
> +    VisiblePosition next = nextBoundary(startingPosition, nextWordPositionBoundary);

next is a very vague name.  In this function, we're concerned about word boundary and on which side of the word boundary we're at.  So your variable name should reflect that.  nextPrev, nextNextPrev do not convey such information and should therefore be be avoided.

> LayoutTests/ChangeLog:8
> +        My change fixed this failure.

You should explain what has been fixed and why it has been fixed.

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