[Webkit-unassigned] [Bug 69753] [GTK] Initial UI client implementation for WebKit2 GTK +API

Mon Dec 12 07:14:44 PST 2011

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

Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #113313|review?                     |review+
               Flag|                            |

--- Comment #14 from Martin Robinson <mrobinson at webkit.org>  2011-12-12 07:14:44 PST ---
(From update of attachment 113313)
View in context: https://bugs.webkit.org/attachment.cgi?id=113313&action=review

Looks good to me. I have a couple documentation nits below as well as a request to use skip the soup server, if possible.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:163
> +static gboolean webkitWebViewAccumulatorObjectHandled(GSignalInvocationHint*, GValue* returnValue, const GValue* handlerReturn, gpointer)
> +{
> +    gpointer object = g_value_get_object(handlerReturn);
> +    if (object)
> +        g_value_set_object(returnValue, object);
> +
> +    return !object;

I prefer void* to gpointer.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:236
> +     * The signal handlers should not try to deal with the reference count for
> +     * the new #WebKitWebView. The widget to which the widget is added will
> +     * handle that.

I find this a bit confusing still. From what I understand, the user will add the widget to a container. The container will handle the reference count. The user is still responsible for adding the widget to the container. They can do this at any time during the process of create -> ready-to-show -> close, right? I think the documentation should just explain that the user should add the WebKitWebView to a container as usual.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:258
> +     * size, position, whether the location, status and scrollbars
> +     * should be displayed, is already set.

Later we should come back and explain where it's set.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:276
> +     * delete the #WebKitWebView, if necessary.

delete -> destroy

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:99
> +    static const char* htmlOnLoadFormat = "<html><body onLoad=\"%s\"></body></html>";
> +
> +    if (message->method != SOUP_METHOD_GET) {
> +        soup_message_set_status(message, SOUP_STATUS_NOT_IMPLEMENTED);
> +        return;
> +    }
> +
> +    soup_message_set_status(message, SOUP_STATUS_OK);
> +
> +    if (g_str_equal(path, "/create_new_view")) {
> +        GOwnPtr<char> windowOpenString(g_strdup_printf("window.open('%s')", kServer->getURIForPath("/new_window").data()));
> +        char* htmlString = g_strdup_printf(htmlOnLoadFormat, windowOpenString.get());
> +        soup_message_body_append(message->response_body, SOUP_MEMORY_TAKE, htmlString, strlen(htmlString));
> +    } else if (g_str_equal(path, "/new_window")) {
> +        char* htmlString = g_strdup_printf(htmlOnLoadFormat, "self.close();");
> +        soup_message_body_append(message->response_body, SOUP_MEMORY_TAKE, htmlString, strlen(htmlString));
> +    }

Can you load a string now instead of using Soup here?

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