[webkit-reviews] review requested: [Bug 57543] -webkit-visual-word does not work on words separated by multiple spaces : [Attachment 93675] patch w/ layout test
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon May 16 11:20:25 PDT 2011
Xiaomei Ji <xji at chromium.org> has asked for review:
Bug 57543: -webkit-visual-word does not work on words separated by multiple
spaces
https://bugs.webkit.org/show_bug.cgi?id=57543
Attachment 93675: patch w/ layout test
https://bugs.webkit.org/attachment.cgi?id=93675&action=review
------- Additional Comments from Xiaomei Ji <xji at chromium.org>
(In reply to comment #20)
> (From update of attachment 93152 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=93152&action=review
>
> > Source/WebCore/editing/visible_units.cpp:1183
> > + // "abc |def | hij |opq". We are traversing these boxes from
right to left. The word
> > + // break between "def" and "hij" (which is position 8) is not in
box "hij opq", and it
> > + // should be added as a word break (the rightmost word break) when
traversing box "abc def".
>
> It's still not clear which of these two boxes correspond to this "box"
(current one).
Changed to:
// "abc |def | hij |opq". We are traversing these boxes from right
to left. When
// traversing box "abc def", the word break between "def" and "hij"
(which is position 8)
// is added as the rightmost word break.
>
> > Source/WebCore/editing/visible_units.cpp:1277
> > + if (wordBreak == previousWordBreak)
> > + return VisiblePosition();
> > +
>
> Why do we need this early exit?
You are right. They are actually not needed.
>
> > Source/WebCore/editing/visible_units.cpp:1327
> > + offsetOfWordBreak = offset;
> > + isLastWordBreakInBox = false;
> > + previousWordBreakWasAtBoxStart = true;
> > + return positionAfterWord;
>
> This makes me think that we might want to bundle up all these functions as a
class.
that could be a good idea. Filed https://bugs.webkit.org/show_bug.cgi?id=60910
for record.
More information about the webkit-reviews
mailing list