[webkit-reviews] review denied: [Bug 104484] [GTK] Add sections documentation to WebKit2 GTK+ API : [Attachment 178423] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Simon Pena
<spena at igalia.com>'s request for review:
Bug 104484: [GTK] Add sections documentation to WebKit2 GTK+ API
https://bugs.webkit.org/show_bug.cgi?id=104484

Attachment 178423: Patch
https://bugs.webkit.org/attachment.cgi?id=178423&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=178423&action=review


Nice! I have a few small suggestions.

> Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardListItem.cpp:37
> + * A history item consists out of a title and a uri. It can be part of

uri-> URI

> Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardListItem.cpp:39
> + * the #WebKitBackForwardList and the global history. The global
> + * history is used for coloring the links of visited sites.

Does WebKit2 have the concept of the global history as well?

> Source/WebKit2/UIProcess/API/gtk/WebKitDownload.cpp:46
> + * and do it itself.

maybe "handle the download process itself" ? "it itself" is a bit hard to read.


> Source/WebKit2/UIProcess/API/gtk/WebKitError.cpp:36
> + * final user, trying to work around them, or taking any other

What do you mean by "final user" ? It seems that this section could probably
just be omitted. The name WebKitError is fairly self-explanatory.

> Source/WebKit2/UIProcess/API/gtk/WebKitFaviconDatabase.cpp:46
> + * WebKit will automatically look for available icons in link elements

You probably want to change link to be &lt;link&gt; for the sake of clarity.

> Source/WebKit2/UIProcess/API/gtk/WebKitFaviconDatabase.cpp:51
> + * If #WebKitSettings:enable-private-browsing is %TRUE new icons won't

Nit: missing a comma after TRUE.

> Source/WebKit2/UIProcess/API/gtk/WebKitPlugin.cpp:35
> + * various usual directories. This object can be used to get more

instead of "various usual directories" maybe you should say "various platform
plugin directcories".

> Source/WebKit2/UIProcess/API/gtk/WebKitURIResponse.cpp:39
> + * status code, the content length, the mime type, the https status or

https status -> HTTP status

> Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:39
> + * JavaScriptDebugger. Using this class one can get a #GtkWidget which

JavaScript debugger

> Source/WebKit2/UIProcess/API/gtk/WebKitWebResource.cpp:40
> + * A web resource encapsulates the data of the download, as well as
> + * the URI and the #WebKitURIResponse.

The use of the word "download" here might be misleading. WebKitWebResource
don't just describe downloads, but they describe a particular resource at the
end of a particular URL. Loading a page will trigger the creation one one
resource for every separate image and stylesheet on the page.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:77
> + * API. #WebKitWebView is scrollable by itself, so you don't need to
> + * embed it in a #GtkScrolledWindow. It is responsible for managing

You can probably omit the discussion about GtkScrolledWindow.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:78
> + * the drawing of the content, forwarding of events. You can load any

drawing of the content and forwarding of events

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:79
> + * URI into the #WebKitWebView or any kind of data string. With

Maybe just omit "any kind" here

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:84
> +/**
> + * SECTION: WebKitWebViewBase
> + * @Short_description: The base class for #WebKitWebView
> + * @Title: WebKitWebViewBase
> + *
> + * Base class for #WebKitWebView.
> + *
> + */
> +

Maybe this should be omitted from the documentation entirely?