WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
43996
Add ability to count text matches without marking
https://bugs.webkit.org/show_bug.cgi?id=43996
Summary
Add ability to count text matches without marking
Sam Weinig
Reported
2010-08-13 16:29:54 PDT
Add ability to count text matches without marking.
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
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Sam Weinig
Comment 1
2010-08-13 16:32:14 PDT
Created
attachment 64387
[details]
Patch
David Kilzer (:ddkilzer)
Comment 2
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.
Sam Weinig
Comment 3
2010-08-31 10:43:42 PDT
Created
attachment 66072
[details]
Updated Patch
WebKit Review Bot
Comment 4
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.
Darin Adler
Comment 5
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.
Sam Weinig
Comment 6
2010-08-31 15:10:39 PDT
Landed in
r66544
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug