Bug 43996 - Add ability to count text matches without marking
Summary: Add ability to count text matches without marking
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-13 16:29 PDT by Sam Weinig
Modified: 2010-08-31 15:10 PDT (History)
1 user (show)

See Also:


Attachments
Patch (9.80 KB, patch)
2010-08-13 16:32 PDT, Sam Weinig
ddkilzer: review-
Details | Formatted Diff | Diff
Updated Patch (12.62 KB, patch)
2010-08-31 10:43 PDT, Sam Weinig
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2010-08-13 16:29:54 PDT
Add ability to count text matches without marking.
Comment 1 Sam Weinig 2010-08-13 16:32:14 PDT
Created attachment 64387 [details]
Patch
Comment 2 David Kilzer (:ddkilzer) 2010-08-13 17:13:10 PDT
Comment on attachment 64387 [details]
Patch

WebCore/page/Frame.cpp:1257
 +  unsigned Frame::markAllMatchesForText(const String& target, bool caseFlag, unsigned limit)

This method should be removed.  Since WebKit is the only caller, it should switch to using countMatchesForText() and pass in true for markMatches.  The WebCore.exp.in file will need to be updated to remove WebCore::Frame::markAllMatchesForText() and to add WebCore::Frame::countMatchesForText().


WebCore/page/Frame.h:211
 +          unsigned markAllMatchesForText(const String&, bool caseFlag, unsigned limit);

This declaration can be removed.


WebKit/mac/WebView/WebHTMLView.mm:6206
 +  - (NSUInteger)countMatchesForText:(NSString *)string caseSensitive:(BOOL)caseFlag limit:(NSUInteger)limit markMatches:(BOOL)markMatches

This method should call WebCore::Frame::countMatchesForText() instead of WebCore::Frame::markAllMatchesForText() if it uses it.


WebKit/mac/WebView/WebPDFView.mm:633
 +  - (NSUInteger)countMatchesForText:(NSString *)string caseSensitive:(BOOL)caseFlag limit:(NSUInteger)limit markMatches:(BOOL)markMatches

This method should call WebCore::Frame::countMatchesForText() instead of WebCore::Frame::markAllMatchesForText() if it uses it.


WebKit/mac/WebView/WebView.mm:4368
 +  - (NSUInteger)countMatchesForText:(NSString *)string caseSensitive:(BOOL)caseFlag highlight:(BOOL)highlight limit:(NSUInteger)limit markMatches:(BOOL)markMatches

This method should call WebCore::Frame::countMatchesForText() instead of WebCore::Frame::markAllMatchesForText() if it uses it.


r- to address the above issues.
Comment 3 Sam Weinig 2010-08-31 10:43:42 PDT
Created attachment 66072 [details]
Updated Patch
Comment 4 WebKit Review Bot 2010-08-31 10:45:04 PDT
Attachment 66072 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/page/Page.cpp:529:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Darin Adler 2010-08-31 10:47:29 PDT
Comment on attachment 66072 [details]
Updated Patch

> +        // Do a "fake" paint in order to execute the code that computes the rendered rect for
> +        // each text match.

Merge comment into one line?

> +        Document* doc = document();
> +        if (m_view && contentRenderer()) {
> +            doc->updateLayout(); // Ensure layout is up to date.

Get rid of pointless local variable "doc"?

> -        unsigned markAllMatchesForText(const String&, bool caseFlag, unsigned limit);
> +        unsigned countMatchesForText(const String&, bool caseFlag, unsigned limit, bool markMatches);

A boolean for this is not great, since we’re often passing a constant. I suppose we can live with this, but it’s ugly.
Comment 6 Sam Weinig 2010-08-31 15:10:39 PDT
Landed in r66544.