[webkit-reviews] review cancelled: [Bug 16222] [GTK] Implement inline search and highlighting of matching strings. : [Attachment 17654] Fixed style issues, renamed string to text

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 5 08:18:19 PST 2007


Mark Rowe (bdash) <mrowe at apple.com> has cancelled '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 17654: Fixed style issues, renamed string to text
http://bugs.webkit.org/attachment.cgi?id=17654&action=edit

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
The use of so many boolean parameters on Page::findString makes me
uncomfortable.	It will make the call sites hard to interpret without referring
to the headers.

The argument names in the header need to be named as their meanings are not at
all obvious based on the context.

I think the !!s should be removed as they were a Windows-ism.

The "found" variable in Page::findString looks to be redundant and could be
removed with a tiny bit of tweaking, without losing any clarity.

The ternary operator in Page::markAllMatchesForText could do with a little bit
of cleanup -- the spacing looks non-standard.


More information about the webkit-reviews mailing list