[webkit-reviews] review denied: [Bug 16222] [GTK] Implement inline
search and highlighting of matching strings. : [Attachment
17650] search and highlighting logic in WebCore
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Dec 2 10:03:25 PST 2007
Adam Roben <aroben at apple.com> has denied Christian Dywan
<christian at twotoasts.de>'s request for review:
Bug 16222: [GTK] Implement inline search and highlighting of matching strings.
http://bugs.webkit.org/show_bug.cgi?id=16222
Attachment 17650: search and highlighting logic in WebCore
http://bugs.webkit.org/attachment.cgi?id=17650&action=edit
------- Additional Comments from Adam Roben <aroben at apple.com>
207 static Frame *incrementFrame(Frame *curr, bool forward, bool wrapFlag)
The *s are misplaced here -- they should go next to the type.
214 bool Page::findString(const String& target, bool forward, bool caseFlag,
bool wrapFlag)
I think caseFlag should be called caseSensitive and wrapFlag should be called
wrap or shouldWrap.
244 uint Page::markAllMatchesForText(const String& target, bool caseSensitive,
bool highlight, uint limit)
You should use unsigned instead of uint.
249 uint matches = 0;
Ditto.
102 bool findString(const String&, bool, bool, bool);
Can this method be const?
It would be nice to change some of the do-while loops into for loops, but
that's optional for this patch.
r- to fix these issues, then let's get this in! Thanks for putting this down in
WebCore.
More information about the webkit-reviews
mailing list