[Webkit-unassigned] [Bug 60910] might need to bundle the functions related to visual word break as a class (VisualWordBreaker)

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Ryosuke Niwa <rniwa at webkit.org>  2011-07-26 12:37:05 PST ---
(From update of attachment 101542)
View in context: https://bugs.webkit.org/attachment.cgi?id=101542&action=review

I'm not sure if this patch is an improvement or not.  It appears that all we need is RenderedPosition proposed in https://docs.google.com/document/d/1vzzi_wQHO0GtLnu-1IYutUKbcpAYlHqZwQlwbfLuowM/edit?hl=en_US&authkey=CLOa9cgN

> Source/WebCore/ChangeLog:3
> +        might need to bundle the functions related to visual word break as a class (VisualWordBreaker)

We should get rid of "might need to" from the bug title now that we're doing it.

> Source/WebCore/editing/visible_units.cpp:1188
> +        static const int s_invalidOffset = -1;

Again, I don't think we should prefix this constant with "s_" as it's not a static "variable".

> Source/WebCore/editing/visible_units.cpp:1190
> +        struct WordBoundaryEntry;

You can just move the definition of WordBoundaryEntry here since it shouldn't be visible outside of this class.

> Source/WebCore/editing/visible_units.cpp:1521
> +int VisualWordBreakWindows::greatestValueUnder()

Now this function name is very weird.  The greatest value under what?  We need to rename this function so that the semantics will be clear.

> Source/WebCore/editing/visible_units.cpp:1546
> -static int smallestOffsetAbove(int offset, bool boxAndBlockAreInSameDirection, const WordBoundaryVector& orderedWordBoundaries)
> +int VisualWordBreakWindows::smallestOffsetAbove()

Ditto.

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