[webkit-reviews] review denied: [Bug 82643] [BlackBerry] Expose CaseSensitive, Wrap, and HighlightAllMatches in WebPage::findNextString() : [Attachment 135380] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 3 12:06:56 PDT 2012
Rob Buis <rwlbuis at gmail.com> has denied Mike Lattanzio <mlattanzio at rim.com>'s
request for review:
Bug 82643: [BlackBerry] Expose CaseSensitive, Wrap, and HighlightAllMatches in
WebPage::findNextString()
https://bugs.webkit.org/show_bug.cgi?id=82643
Attachment 135380: Patch
https://bugs.webkit.org/attachment.cgi?id=135380&action=review
------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=135380&action=review
Looks good, can be improved a bit still.
> Source/WebKit/blackberry/WebKitSupport/InPageSearchManager.cpp:172
> + if (match) {
Can combine ttese two lines.
> Tools/DumpRenderTree/blackberry/LayoutTestControllerBlackBerry.cpp:815
> WTF::String nameStr = jsStringRefToWebCoreString(target);
Probably no need for WTF::
> Tools/DumpRenderTree/blackberry/LayoutTestControllerBlackBerry.cpp:838
> + return
BlackBerry::WebKit::DumpRenderTree::currentInstance()->page()->findNextString(n
ameStr.utf8().data(), !(options & WebCore::Backwards), !(options &
WebCore::CaseInsensitive), true, true);
You probably want to explain what true params do here (true /* wrap */ etc).
More information about the webkit-reviews
mailing list