[webkit-reviews] review granted: [Bug 129586] Find results on simple lines are not marked correctly : [Attachment 225662] rebased

[bugzilla-daemon at webkit.org]

Andreas Kling <akling at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 129586: Find results on simple lines are not marked correctly
https://bugs.webkit.org/show_bug.cgi?id=129586

Attachment 225662: rebased
https://bugs.webkit.org/attachment.cgi?id=225662&action=review

------- Additional Comments from Andreas Kling <akling at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=225662&action=review


r=me

> Source/WebCore/editing/TextIterator.cpp:544
> -	   while (runEnd < str.length() &&
(deprecatedIsCollapsibleWhitespace(str[runEnd]) || str[runEnd] == '\t'))
> +	   while (runEnd < end &&
(deprecatedIsCollapsibleWhitespace(str[runEnd]) || str[runEnd] == '\t'))

It irks me that we are calling operator[] on WTF::String in a loop instead of
at least just operating on the StringImpl, but this is hardly time||place for
that kinda cleanup.

> Source/WebCore/editing/TextIterator.cpp:1304
> -    if (!renderer->firstTextBox() && text.length() > 0)
> +    if (!renderer->hasRenderedText() && text.length() > 0)

We should make the fat linebox getters assert that we are not in simple linebox
mode.
(Or better yet, abstract away the ability to call them in the wrong context by
encapsulating lines.)