[Webkit-unassigned] [Bug 59143] Need populate touch-icon url to FrameLoaderClient
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 26 11:27:29 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=59143
--- Comment #5 from David Kilzer (ddkilzer) <ddkilzer at webkit.org> 2011-04-26 11:27:28 PST ---
(In reply to comment #4)
> The inline comment didn't show up, copied it here.
>
> Hi David,
>
> Thanks very much for the quick response.
>
> I have some issues want to be discussed before I make another patch.
>
> a. Is it OK that using 'SUPPORT_TOUCH_ICON' to disable the touch icon loading?
It's not so much about support as it is whether you want to load the touch icons during a page load. Maybe ENABLE(TOUCH_ICON_LOADING)?
> b. Please refer to my inline reply for convertIconTypeToIndex.
>
>
> Tao
>
> >
> > > Source/WebCore/dom/Document.cpp:355
> > > + default:
> > > + return -1;
> >
> > There should not be a default case here. Having one defeats the compiler from warning you when you don't have a case defined. This return statement should be moved outside of the switch statement.
> >
> > This should also be an (inline) method in IconURL.{h|cpp} so that you can share code between Document.cpp and DocumentLoader.cpp.
> >
> > This method should really return size_t (which is used for array indexes), and then default to Favicon if 'type' is unknown and ASSERT_NOT_REACHED() before returning a default value.
> >
>
> The index of a array is the implementation specific thing, and this code might not need to be shared. To put this method in IconURL looks a little weird, isn't it? Anyway, I am not familiar with Webkit convention, If you think it is fine, I am also OK with it. Please let me know.
>
> Does ASSERT_NOT_REACHED work in debug build? Shall we return 0 (Favicon) in release build if 'type' is invalid?
ASSERT_NOT_REACHED() will crash a Debug build, but do nothing on Release builds. Returning Favicon sounds good (since that was the behavior before adding the touch icon support).
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list