Summary: | [BlackBerry] Expose CaseSensitive, Wrap, and HighlightAllMatches in WebPage::findNextString() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mike Lattanzio <mlattanzio> | ||||||
Component: | WebKit BlackBerry | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | tonikitoo, webkit.review.bot | ||||||
Priority: | P3 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | Other | ||||||||
Attachments: |
|
Description
Mike Lattanzio
2012-03-29 12:44:20 PDT
Created attachment 135380 [details]
Patch
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 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 Created attachment 135392 [details]
Patch
Comment on attachment 135392 [details]
Patch
Looks good.
Comment on attachment 135392 [details] Patch Clearing flags on attachment: 135392 Committed r113073: <http://trac.webkit.org/changeset/113073> All reviewed patches have been landed. Closing bug. |