[webkit-reviews] review denied: [Bug 60247] Handle the touch icon in WebKit/Chromium : [Attachment 92836] Address the comment

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 10 08:23:22 PDT 2011


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied  review:
Bug 60247: Handle the touch icon in WebKit/Chromium
https://bugs.webkit.org/show_bug.cgi?id=60247

Attachment 92836: Address the comment
https://bugs.webkit.org/attachment.cgi?id=92836&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92836&action=review

> Source/WebKit/chromium/public/WebFrame.h:135
> +    virtual WebVector<WebIconURL> favIconURL(int iconTypes) const = 0;

you should explain what this mysterious int parameter is.  it is not obviously
a bit-mask of Type enum values.  you should say so!

> Source/WebKit/chromium/public/WebIconURL.h:40
> +    enum WebIconType {

because this enum is nested inside a class, it does not need the redundant
WebIcon prefix.  if you look around at other headers, you will see that this
should just be enum Type { TypeInvalid = 0, ... };

> Source/WebKit/chromium/src/WebFrameImpl.cpp:529
> +	   WTF::Vector<WebCore::IconURL> iconURLs =
frameLoader->iconURLs(webIconTypes);

no WTF:: in .cpp files


More information about the webkit-reviews mailing list