Bug 82643 - [BlackBerry] Expose CaseSensitive, Wrap, and HighlightAllMatches in WebPage::findNextString()
Summary: [BlackBerry] Expose CaseSensitive, Wrap, and HighlightAllMatches in WebPage::...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P3 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-29 12:44 PDT by Mike Lattanzio
Modified: 2012-04-03 13:02 PDT (History)
2 users (show)

See Also:


Attachments
Patch (21.93 KB, patch)
2012-04-03 11:51 PDT, Mike Lattanzio
no flags Details | Formatted Diff | Diff
Patch (22.04 KB, patch)
2012-04-03 12:23 PDT, Mike Lattanzio
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Lattanzio 2012-03-29 12:44:20 PDT
Plumb through some additional parameters to improve find.
Comment 1 Mike Lattanzio 2012-04-03 11:51:07 PDT
Created attachment 135380 [details]
Patch
Comment 2 Rob Buis 2012-04-03 12:06:56 PDT
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).
Comment 3 Antonio Gomes 2012-04-03 12:10:15 PDT
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
Comment 4 Mike Lattanzio 2012-04-03 12:23:21 PDT
Created attachment 135392 [details]
Patch
Comment 5 Rob Buis 2012-04-03 12:24:54 PDT
Comment on attachment 135392 [details]
Patch

Looks good.
Comment 6 WebKit Review Bot 2012-04-03 13:02:45 PDT
Comment on attachment 135392 [details]
Patch

Clearing flags on attachment: 135392

Committed r113073: <http://trac.webkit.org/changeset/113073>
Comment 7 WebKit Review Bot 2012-04-03 13:02:50 PDT
All reviewed patches have been landed.  Closing bug.