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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 13 03:54:38 PST 2011


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





--- Comment #15 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-12-13 03:54:38 PST ---
(In reply to comment #14)
> (From update of attachment 113313 [details])
> 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.

Ok

> > 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.

I agree, the fact that we are adding that comment makes me think that it's a special situation, while it isn't. It's like any other widget creation, the widget contains a floating reference, so I think we should just remove the comment. 

> > 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.

Yes.

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

Ok.

> > 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?

Yes, I'll rework the tests to use html.

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



More information about the webkit-unassigned mailing list