[Webkit-unassigned] [Bug 57543] -webkit-visual-word does not work on words separated by multiple spaces

[bugzilla-daemon at webkit.org]

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





--- Comment #17 from Ryosuke Niwa <rniwa at webkit.org>  2011-05-10 18:35:37 PST ---
(From update of attachment 93048)
View in context: https://bugs.webkit.org/attachment.cgi?id=93048&action=review

> Source/WebCore/editing/visible_units.cpp:1178
> +        // it is not covered by the box previously visited. For example, given a logical text 

Nit: if it is not in the previously visited boxes.

> Source/WebCore/editing/visible_units.cpp:1179
> +        // "abc def     hij opq", there are 2 boxes "abc def " (starts at 0 and length is 8) and 

Nit: You forgot a preposition before "abc...".  In "abc.."?

> Source/WebCore/editing/visible_units.cpp:1182
> +        // "hij opq" (starts at 12 and length is 7). The word breaks are at position 3 (which is 
> +        // right after "abc"), position 8 (which is right after "def "), and position 15 (which is 
> +        // right after "hij"). Word break betwen "def" and "hij" (which is position 8) does not 

This paragraph is really verbose.  Can we just use | notation?  I think people get what you mean.

Also maybe we should state in which order we're traversing these boxes and which one is the current box and which one is the next leaf child.

> Source/WebCore/editing/visible_units.cpp:1297
> +    const InlineBox* box, const VisiblePosition& previousWordBreak, int& offsetOfWordBreak, bool& isLastWordBreakInBox, bool& previousWordBreakIsInBoundary)

Nit: s/previousWordBreakIsInBoundary/previousWordBreakWasAtBoundary/

In addition, it'll be nice to say whether it's at start (left for LTR/right for RTL) or end (right for LTR/left for RTL).

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

> Source/WebCore/editing/visible_units.cpp:1178
> +        // it is not covered by the box previously visited. For example, given a logical text 

Nit: if it is not in the previously visited boxes.

> Source/WebCore/editing/visible_units.cpp:1179
> +        // "abc def     hij opq", there are 2 boxes "abc def " (starts at 0 and length is 8) and 

Nit: You forgot a preposition before "abc...".  In "abc.."?

> Source/WebCore/editing/visible_units.cpp:1182
> +        // "hij opq" (starts at 12 and length is 7). The word breaks are at position 3 (which is 
> +        // right after "abc"), position 8 (which is right after "def "), and position 15 (which is 
> +        // right after "hij"). Word break betwen "def" and "hij" (which is position 8) does not 

This paragraph is really verbose.  Can we just use | notation?  I think people get what you mean.

Also maybe we should state in which order we're traversing these boxes and which one is the current box and which one is the next leaf child.

> Source/WebCore/editing/visible_units.cpp:1297
> +    const InlineBox* box, const VisiblePosition& previousWordBreak, int& offsetOfWordBreak, bool& isLastWordBreakInBox, bool& previousWordBreakIsInBoundary)

Nit: s/previousWordBreakIsInBoundary/previousWordBreakWasAtBoundary/

In addition, it'll be nice to say whether it's at start (left for LTR/right for RTL) or end (right for LTR/left for RTL).

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