[Webkit-unassigned] [Bug 60247] Handle the touch icon in WebKit/Chromium

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60247





--- Comment #36 from michaelbai at chromium.org  2011-05-11 14:04:08 PST ---
(In reply to comment #34)
> (From update of attachment 93147 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=93147&action=review
> 
> > Source/WebKit/chromium/WebKit.gyp:188
> > +                'public/WebIconType.h',
> 
> wrong file?
> 

Done.

> > Source/WebKit/chromium/public/WebFrame.h:138
> > +    // WebIconURL::Type values, sed to select from the available set of icon
> 
> "sed" -> used?
> 

Done.

> > Source/WebKit/chromium/public/WebIconURL.h:62
> > +    WebIconURL(const WebCore::IconURL& iconURL)
> 
> nit: we usually place any WEBKIT_IMPLEMENTATION section as the very end of the public section.  take a look at other header files to familiarize yourself with conventions.
> 

Sorry, I am anxious to get this check in. 

> > Source/WebKit/chromium/src/WebFrameImpl.cpp:537
> > +        Vector<WebCore::IconURL> iconURLs = frameLoader->iconURLs(webIconTypes);
> 
> were you not able to replace all of this code with a simple "return frameLoader->iconURLs(...);"
> 
> ^^^ that was the point of defining the constructor

Not aware of the magic here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.