[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





--- Comment #35 from Van Lam <vanlam at google.com>  2011-08-16 09:58:50 PST ---
(In reply to comment #34)
> (From update of attachment 103969 [details])
> 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):

As far as visual word movement goes it seemed like the implementation in TextBoundaries.cpp works on Mac; but to make the change uniform across platforms I'll add the duplication.

> 
> 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?

Good catch; there only needs to be one copy of findNextWordFromIndex that delegates to the platform's 
 findNextWordFromIndexConsideringWordBreakPositioningRelativeToWord.

Thanks for the review.

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