[Webkit-unassigned] [Bug 98885] [GTK] It should be possible to disable favicons in WebKit2 GTK+ API

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from Mario Sanchez Prada <msanchez at igalia.com>  2012-10-10 06:29:20 PST ---
I agree with the idea, yet I have some comments about the specific implementation for it. See them below...

(In reply to comment #0)
> [...]
> So, to enable the favicons, the user needs to call 
> webkit_web_context_set_favicon_database_directory(). If it hans't been called,
> web view will use the database, that will return
> WEBKIT_FAVICON_DATABASE_ERROR_NOT_INITIALIZED every time it's asked for the
> favicon. 

If we are going to force that calling to this method is necessary to enable the favicon database (will be disabled by default) and this method will only do stuff make sense if called once, then I think it would make sense to rename it to something like this:

  void webkit_web_context_initialize_favicon_database(const char* path);

This would be also more consistent with the NOT_INITIALIZED error that would be issuing when trying to use the icon database before calling this method.

In case we wanted to support re-setting the path of the database later on (once it's allowed in WK2), it would be as easy as adding the method webkit_web_context_set_database_directory() again, which would do something like this:

  1. Close the current IconDatabase
  2. Set the path as the new directory to be used
  3. Open the IconDatabase again.

> This way the favicon database will never emit the favicon-changed
> signal and web view won't emit notify::favicon. In the worst case we have a
> WebKitFaviconDatabase object that does nothing but we don't need special
> case when the database is not open, and the web view code will be the same.

Yes. Makes perfect sense to me.

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