[Webkit-unassigned] [Bug 142786] [GTK] webkit_web_extension_initialize[_with_user_data] not documented

[bugzilla-daemon at webkit.org]

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

Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #250778|review+, commit-queue+      |review?, commit-queue-
              Flags|                            |

--- Comment #13 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 250778
  --> https://bugs.webkit.org/attachment.cgi?id=250778
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=250778&action=review

> Source/WebKit2/ChangeLog:8
> +        WebKitWebExtension has no documentation at all the only information

no documentation at all is at least inaccurate, WebKitWebExtension has a signal and a public method and both are documented. Initialization functions are also documented in WebKitWebExtension.h

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:37
> + * SECTION: WebKitWebExtension

So, the section documentation is what is missing then.

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:41
> + * #WebKitWebExtension is a plugin for the WebProcess. It allows you to execute code in the

Do not use # here, since this creates a link to this point exactly. I would avoid using 'plugin' here, since it could be confused with browser plugins. I would use loadable module, for example.

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:45
> + * WebExtensions are a single compilation unit that are loaded in a way similar to a GTK module.

I wouldn't mention GTK modules here either, I think previous paragraph already explained that this is loadable module loaded by the web process.

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:46
> + * To create a #WebKitWebExtension you should write a module which implements a function

Do not use # here.

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:47
> + * named `webkit_web_extension_initialize` or, in case you need to initilize the extension with

Use webkit_web_extension_initialize() instead of the quotes

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:48
> + * some data from the UI, `webkit_web_extension_initialize_with_user_data`. These functions

webkit_web_extension_initialize_with_user_data()

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:49
> + * types are WebKitWebExtensionInitializeFunction() and WebKitWebExtensionInitializeWithUserDataFunction()

WebKitWebExtensionInitializeFunction and WebKitWebExtensionInitializeWithUserDataFunction are not functions, use # instead of ().

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:50
> + * respectively. In addition the implemented function has to be public and it has to use

I would rephrase this a bit, something like. Web extensions must have a initialization function that could be either webkit_web_extension_initialize() with prototype #WebKitWebExtensionInitializeFunction or webkit_web_extension_initialize_with_user_data() with prototype #WebKitWebExtensionInitializeWithUserDataFunction. This function is called when the web process is initialized.

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:51
> + * the `G_MODULE_EXPORT` macro.

#G_MODULE_EXPORT

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:71
> + * G_MODULE_EXPORT void
> + * webkit_web_extension_initialize_with_user_data (WebKitWebExtension *extension,
> + *                                                 const GVariant     *user_data)
> + * {
> + *     g_signal_connect (extension, "page-created",
> + *                       G_CALLBACK (web_page_created_callback),
> + *                       NULL);
> + * }

If we are not going to use the data in the example, it would be better to use webkit_web_extension_initialize() instead.

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:76
> + * a #WebKitWebPage is created. You should also modify your building tools (e.g. CMake or
> + * Autotools) to compile this individual module.

I'm not sure we should mention build systems in api docs.

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:78
> + * WebKit has to know where it can find the created #WebKitWebExtension. To do so you

Dot not use # here.

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:80
> + * must be called as soon as posible in our app and before any WebProcess is launched.

in our app? I would just mention initialize-web-extensions signal as the recommended place to avoid confusions.

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:83
> + * To provide the initialization data used by the `webkit_web_extension_initialize_with_user_data`

I guess you provide init data to the function, not by the function. In any case, I think Martin or any other native english speaker should review all this doc.

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:100
> + *       context, WEB_EXTENSIONS_DIRECTORY);
> + *   webkit_web_context_set_web_extensions_initialization_user_data (
> + *      context, g_variant_new_uint32 (unique_id++));

It seems one of these lines is not correctly indented

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150520/68f48a8b/attachment-0001.html>