[Webkit-unassigned] [Bug 98874] [GTK] WebKitWebView doesn't notify of favicon changes for known favicons but new pages

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Nov 5 12:27:49 PST 2012


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
 Attachment #168732|review?                     |review+
               Flag|                            |

--- Comment #19 from Martin Robinson <mrobinson at webkit.org>  2012-11-05 12:29:18 PST ---
(From update of attachment 168732)
View in context: https://bugs.webkit.org/attachment.cgi?id=168732&action=review

This looks sane to me. I don't think we should block the patch any longer. If it needs to be tested in epiphany, we can always land after that's finished.

>> Source/WebKit2/UIProcess/API/gtk/WebKitFaviconDatabase.cpp:108
>> +                     G_TYPE_STRING,
> Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]

Please fix the style here before landing.

>>> Source/WebKit2/UIProcess/API/gtk/WebKitFaviconDatabase.cpp:195
>>> +
>> Also as commented on jabber, seems we are never notified when a favicon changen but its URIs stays the same. Not sure where this could be added as a FIXME, but it would be good to document somewhere this kind of thing.
> The API doc for favicon-changed signals says: "This signal is emitted when the favicon URI of @page_uri has been changed to @favicon_uri in the database." so it's documented somewhere :-)
> I was assuming we were not notified when the icon data changes, because this callback is never called for this page http://www.p01.org/releases/DEFENDER_of_the_favicon/ that changes the favicon dynamically, but I haven't tried other test cases, so I can be wrong.

I think this is an interesting question to answer indeed. The callback name suggests that it should be called when the data changes, but not the URL. How exactly do we plan to support pages like http://www.p01.org/releases/DEFENDER_of_the_favicon/ if not with the favicon-changed signal? If this will never be called when the URL is the same perhaps it's better named favicon-uri-changed.

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