[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