[webkit-reviews] review denied: [Bug 65277] Make functions to find word boundaries more flexible : [Attachment 103969] Proposed fix

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Van Lam <vanlam at google.com>'s
request for review:
Bug 65277: Make functions to find word boundaries more flexible
https://bugs.webkit.org/show_bug.cgi?id=65277

Attachment 103969: Proposed fix
https://bugs.webkit.org/attachment.cgi?id=103969&action=review

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


Why is there no change in platform/text/mac/TextBoundaries.mm?

> Source/WebCore/ChangeLog:19
> +	   (WebCore::previousWordPositionBoundaryLogicallyBeforeWord):
> +	   (WebCore::previousWordPositionBoundaryLogicallyAfterWord):
> +	   (WebCore::nextWordPositionBoundaryLogicallyBeforeWord):
> +	   (WebCore::nextWordPositionBoundaryLogicallyAfterWord):

Before and after are logical concepts so "logically" is redundant.

> Source/WebCore/platform/text/TextBoundaries.cpp:112
> +#if !PLATFORM(BREWMP) && !PLATFORM(MAC) && !PLATFORM(QT)

We shouldn't be duplicating code in Brew and Qt and leaving Mac behind.  r-
because of this.

> Source/WebCore/platform/text/qt/TextBoundariesQt.cpp:82
> +int findNextWordFromIndex(UChar const* buffer, int len, int position, bool
forward)
> +{
> +    return
findNextWordFromIndexConsideringWordBreakPositioningRelativeToWord
> +	   (buffer, len, position, forward, forward ? LogicallyAfterWord :
LogicallyBeforeWord);
> +}
> +

What's the point of duplicating the function here?