[Webkit-unassigned] [Bug 113583] [GTK] Add API for remembering visited links

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Martin Robinson <mrobinson at webkit.org>  2013-03-29 08:23:30 PST ---
(From update of attachment 195746)
View in context: https://bugs.webkit.org/attachment.cgi?id=195746&action=review

Nice patch. What do you plan to use it for? The only issue I see are some small style errors and documentation that could use a little more love.

> Source/WebKit2/ChangeLog:7
> +        https://bugs.webkit.org/show_bug.cgi?id=113583
> +
> +        Reviewed by NOBODY (OOPS!).
> +

This could use small description of the usecase and implementation.

> Source/WebKit2/ChangeLog:19
> +        * GNUmakefile.list.am:
> +        * UIProcess/API/gtk/WebKitHistoryClient.cpp: Added.
> +        (populateVisitedLinks):
> +        (attachHistoryClientToContext):
> +        * UIProcess/API/gtk/WebKitHistoryClient.h: Added.
> +        * UIProcess/API/gtk/WebKitWebContext.cpp:
> +        (createDefaultWebContext):
> +        (webkit_web_context_add_visited_link):
> +        (webkitWebContextPopulateVisitedLinks):
> +        * UIProcess/API/gtk/WebKitWebContext.h:
> +        * UIProcess/API/gtk/WebKitWebContextPrivate.h:
> +        * UIProcess/API/gtk/docs/webkit2gtk-sections.txt:

Normally we put small descriptions of each change after the colons here.

>> Source/WebKit2/UIProcess/API/gtk/WebKitHistoryClient.cpp:30
>> +    WebKitWebContext *webContext;
> 
> Declaration has space between type name and * in WebKitWebContext *webContext  [whitespace/declaration] [3]

Please make sure to fix all style errors. You can verify your patch before uploading it by using the webkit-patch tool in Tools/Scripts or check-webkit-style.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:179
> +     * This signal can be connected to populate the list of
> +     * visited links from an external history database.

This could probably be expanded slightly. Should the user only call webkit_web_context_add_visited_link in this signal handler? If so that should be mentioned both here and in the documentation for webkit_web_context_add_visited_link.

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