Bug 82643 – [BlackBerry] Expose CaseSensitive, Wrap, and HighlightAllMatches in WebPage::findNextString()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Mike Lattanzio]

Plumb through some additional parameters to improve find.

[Mike Lattanzio]

Created attachment 135380 [details]
Patch

[Rob Buis]

Comment on attachment 135380 [details]
Patch

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(nameStr.utf8().data(), !(options & WebCore::Backwards), !(options & WebCore::CaseInsensitive), true, true);

You probably want to explain what true params do here (true /* wrap */ etc).

[Antonio Gomes]

Comment on attachment 135380 [details]
Patch

Lets avoid bools in APIs. See how QtWebKit1 did it: http://doc.qt.nokia.com/4.7-snapshot/qwebview.html#findText

[Mike Lattanzio]

Created attachment 135392 [details]
Patch

[Rob Buis]

Comment on attachment 135392 [details]
Patch

Looks good.

[WebKit Review Bot]

Comment on attachment 135392 [details]
Patch

Clearing flags on attachment: 135392

Committed r113073: <http://trac.webkit.org/changeset/113073>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.