Bug 68074 – [GTK][WEBKIT2] Add support for title property in WebKitWebView

• Component: WebKit2
• Status: RESOLVED FIXED

[Nayan Kumar K]

Add support for title and progress property in WebKitWebView and modify GtkLauncher2 to handle these properties

[Nayan Kumar K]

Created attachment 107318 [details]
Add title and progress properties

[Nayan Kumar K]

With this patch, webkit_web_view_get_title returns g_strdup of the title obtained using title.utf8().data() (As directly returning title.utf8().data() returns garbage). Also it assumes that application calling this function is responsible for freeing this dup'ed memory, which is not consistent with the definition of this API in webkit1. 

One way to solve this would be to maintain title inside WebKitWebViewPrivate. Will that be a good approach to follow?

[Nayan Kumar K]

Created attachment 109188 [details]
Title property added.

Implementation of title property as per the new GObjectization design.

This approach adds 'title' property to WebKitWebView. Loader client will register didTitleReceiveCallback and intimates WebView upon title change. WebView will in turn notifies the application via g_object_notify.

GtkLauncher2 is modified accordingly.

[WebKit Review Bot]

Attachment 109188 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  Extra space before ( in function call  [whitespace/parens] [4]
Tools/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:154:  Missing space before ( in if(  [whitespace/parens] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:298:  webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:305:  Missing space before ( in if(  [whitespace/parens] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:128:  Declaration has space between type name and * in WebKitWebLoaderClient *loaderClient  [whitespace/declaration] [3]
Total errors found: 7 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Comment on attachment 109188 [details]
Title property added.

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

Thanks for the patch!.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:132
> +    webkitWebViewNotifyTitleReceived(webView, title);
> +}

This callback should emit a signal like the other callbacks, and the view can just connect to it to update the title. That way we don't private API to notify the view.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:35
> +#include <wtf/text/WTFString.h>

You are using CString, so include CString.h instead.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:133
> +     * WebKitWebView:title

This should end with :

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:135
> +     * The title associated with WebView's document.

Use the same documentation than webkit1 for things that don't change, this way we have the same strings for translators and API users know this is the same thing.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:150
> +    g_return_if_fail(WEBKIT_IS_WEB_VIEW(webView));

Don't use g_return* macros in private methods.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:296
> + * Returns the #Object:title associated with #WebKitWebView.

It should be Returns: Use the same documentation than webkit1 here too.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:308
> +    WebKitWebViewPrivate* priv = webView->priv;
> +    WebPageProxy* page = webkitWebViewBaseGetPage(WEBKIT_WEB_VIEW_BASE(webView));
> +    String title = page->pageTitle();
> +    if(priv->title != title.utf8())
> +        priv->title = title.utf8();
> +
> +    return priv->title.data();

We are already notified when the title changes, we shouldn't need to check it here again. Or can it change after document is loaded?

> Tools/ChangeLog:10
> +        * GtkLauncher/main.c:
> +        (updateTitle):
> +        (createBrowser):
> +

I wouldn't change the GtkLauncher code. We added this GtkLauncher2 when we wanted to use the same api than webkit1, ideally the same code would work and current #ifdefs were just provisional until new api was added. That's not the case anymore, and we don't want GtkLauncher code with a lot of #ifdefs to support both wk1 and wk2. For now, it's better to add unit tests for every new API. Once we have enough API, we will port MiniBrowser to use the GTK+ API and we'll remove webkit2 support from GtkLauncher.

[Nayan Kumar K]

Comment on attachment 109188 [details]
Title property added.

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

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:132
>> +}
> 
> This callback should emit a signal like the other callbacks, and the view can just connect to it to update the title. That way we don't private API to notify the view.

If I understood correctly, this would mean that, applications can connect to 'title-change' signal on WebKitWebLoaderClient and can be notified when title changes. Also, since WebView in turn notifies about the change in 'title' property, application might get redundant notification if they connect both to 'title-change' signal on WebKitWebLoaderClient or 'notify:title' on WebView. I think, our API's should restrict redundancy. What do you say?

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:35
>> +#include <wtf/text/WTFString.h>
> 
> You are using CString, so include CString.h instead.

I use String also, in webkit_web_view_get_title function.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:308
>> +    return priv->title.data();
> 
> We are already notified when the title changes, we shouldn't need to check it here again. Or can it change after document is loaded?

I think you are right, WebKit should notify if there are any change in the title (even after the document loads). I think its fair enough to assume that this function gets triggered only upon title change.

>> Tools/ChangeLog:10
>> +
> 
> I wouldn't change the GtkLauncher code. We added this GtkLauncher2 when we wanted to use the same api than webkit1, ideally the same code would work and current #ifdefs were just provisional until new api was added. That's not the case anymore, and we don't want GtkLauncher code with a lot of #ifdefs to support both wk1 and wk2. For now, it's better to add unit tests for every new API. Once we have enough API, we will port MiniBrowser to use the GTK+ API and we'll remove webkit2 support from GtkLauncher.

Make sense. I will try to add  unit test for this.

[Nayan Kumar K]

Created attachment 109281 [details]
Title property added.

Incorporated review comments. Added unit test.

[WebKit Review Bot]

Attachment 109281 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:46:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:182:  Declaration has space between * and variable name in SoupServer* server  [whitespace/declaration] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:295:  webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 6 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nayan Kumar K]

Created attachment 109282 [details]
Title property added

Fixed few style issues.

[WebKit Review Bot]

Attachment 109282 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:93:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:93:  The parameter name "web_view" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:104:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:104:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:255:  webkit_web_view_load_html_string is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:304:  webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 6 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nayan Kumar K]

Created attachment 109283 [details]
Title property added to WebKitWebView.

Oops, submitted wrong patch earlier!

[WebKit Review Bot]

Attachment 109283 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:295:  webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

Comment on attachment 109283 [details]
Title property added to WebKitWebView.

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

> Source/WebKit2/ChangeLog:8
> +        Support for 'title' property is added in WebKitWebView.
> +        Functions to get the value of this property is provided.
> +

Typically the description goes under the review line.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:131
> +    webkitWebViewNotifyTitleReceived(webView, title);

This can just be webkitWebViewNotifyTitleReceived(WEBKIT_WEB_LOADER_CLIENT(clientInfo)->priv->view.get(), title);

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:50
>      WebKitWebContext* context;
>  
>      GRefPtr<WebKitWebLoaderClient> loaderClient;
> +    CString title;

Let's keep all the properties and loader clients in separate groups. The order isn't important beyond organization.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:134
> +     * Returns: the @web_view's document title.

I think you should be more explicit here. This is the title of the main frame. also why are you using @web_view there's no parameter. I recommend using #WebKitWebView explicitly instead:

Returns: the main frame title of this #WebKitWebView. If the title has not been received yet it will be an empty string.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:145
> +void webkitWebViewNotifyTitleReceived(WebKitWebView *webView, WKStringRef titleRef)

I'd prefer this to be called webkitWebViewSetTitle and to take a const char*.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:295
>> + * Returns the @web_view's document title.
>> + *
>> + * Returns: the title of @web_view.
>> + */
>> +const gchar* webkit_web_view_get_title(WebKitWebView* webView)
> 
> webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]

Here you have the same problem but it's worse even becaues the name of the paramter is @webView.  We really need a build step which verifies the sanity of the documentation. I'd like to have zero warnings when generating it.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:301
> +
> +    WebKitWebViewPrivate* priv = webView->priv;
> +
> +    return priv->title.data();

This can all be one line.  Ensure that an empty CString does not return null.

>>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99
>>> +webkit_web_view_get_title         (WebKitWebView         *webView);
>> 
>> The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
> 
> Extra space before ( in function call  [whitespace/parens] [4]

Shoudl be *web_view.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewPrivate.h:36
> +G_BEGIN_DECLS
> +
> +void webkitWebViewNotifyTitleReceived(WebKitWebView *, WKStringRef);
> +
> +G_END_DECLS

There's no need to specify that these are unmangled. In fact, it's probably better that private API stays mangled.

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:38
> +#define HTML_TITLE "WelCome to WebKit-GTK+"
> +/* This string has to be rather big because of the cancelled test - it
> + * looks like soup refuses to send or receive a too small chunk */
> +#define HTML_STRING "<html><head><title>"HTML_TITLE"</title></head><body>Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!</body></html>"
> +
> +/* For real request testing */
> +static void serverCallback(SoupServer* server, SoupMessage* msg, const char* path, GHashTable* query, SoupClientContext* context, gpointer data)
> +{
> +    if (msg->method != SOUP_METHOD_GET) {
> +        soup_message_set_status(msg, SOUP_STATUS_NOT_IMPLEMENTED);

Please don't just copy and paste the entire loading test. The test could be quite simple you could do a notify on the title property of WebView, load a simple string and then assert that that the title is what you think it is. Don't forget to assert the proper title before loading anything too.

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:195
> +    server = soup_server_new(SOUP_SERVER_PORT, 0, NULL);
> +    soup_server_run_async(server);
> +
> +    soup_server_add_handler(server, NULL, serverCallback, NULL, NULL);
> +
> +    baseURI = soup_uri_new("http://127.0.0.1/");
> +    soup_uri_set_port(baseURI, soup_server_get_port(server));

I'm almost certain you don't need to make a SoupServer here.

[Nayan Kumar K]

Created attachment 109449 [details]
Added title property

[WebKit Review Bot]

Attachment 109449 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  The parameter name "web_view" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:298:  webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nayan Kumar K]

Created attachment 109452 [details]
Added 'title' property

Incorporated review comments.

[Nayan Kumar K]

Comment on attachment 109283 [details]
Title property added to WebKitWebView.

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

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:131
>> +    webkitWebViewNotifyTitleReceived(webView, title);
> 
> This can just be webkitWebViewNotifyTitleReceived(WEBKIT_WEB_LOADER_CLIENT(clientInfo)->priv->view.get(), title);

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:50
>> +    CString title;
> 
> Let's keep all the properties and loader clients in separate groups. The order isn't important beyond organization.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:134
>> +     * Returns: the @web_view's document title.
> 
> I think you should be more explicit here. This is the title of the main frame. also why are you using @web_view there's no parameter. I recommend using #WebKitWebView explicitly instead:
> 
> Returns: the main frame title of this #WebKitWebView. If the title has not been received yet it will be an empty string.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:145
>> +void webkitWebViewNotifyTitleReceived(WebKitWebView *webView, WKStringRef titleRef)
> 
> I'd prefer this to be called webkitWebViewSetTitle and to take a const char*.

Changed the signature from webkitWebViewNotifyTitleReceived to webkitWebViewSetTitle.

However, if we change the argument to const char*, then we need to cache WebKit::toImpl(titleRef)->string().utf8().data(), send this as argument of webKitWebViewSetTitle and convert it back to CString in webkitWebViewSetTitle implementation, as we copy this variable to WebView property 'title', which is a CString. Since this is a private method, I believe cleaner way to do this is via WKStringRef. Please let me know your opinion.

>>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:295
>>> +const gchar* webkit_web_view_get_title(WebKitWebView* webView)
>> 
>> webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
> 
> Here you have the same problem but it's worse even becaues the name of the paramter is @webView.  We really need a build step which verifies the sanity of the documentation. I'd like to have zero warnings when generating it.

Done. 

Please note that, I tried to keep the documentation same as webkit1 (https://bugs.webkit.org/show_bug.cgi?id=68074#c5). In fact, few of the methods added earlier also refers to @web_view, hence I followed the same. I am in process of writing a patch to generate documentation for webkit2 gtk+ api, I will address rest of such mismatches in that patch.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:301
>> +    return priv->title.data();
> 
> This can all be one line.  Ensure that an empty CString does not return null.

Done.

>>>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99
>>>> +webkit_web_view_get_title         (WebKitWebView         *webView);
>>> 
>>> The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
>> 
>> Extra space before ( in function call  [whitespace/parens] [4]
> 
> Shoudl be *web_view.

Sorry, corrected it now.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewPrivate.h:36
>> +G_END_DECLS
> 
> There's no need to specify that these are unmangled. In fact, it's probably better that private API stays mangled.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:38
>> +        soup_message_set_status(msg, SOUP_STATUS_NOT_IMPLEMENTED);
> 
> Please don't just copy and paste the entire loading test. The test could be quite simple you could do a notify on the title property of WebView, load a simple string and then assert that that the title is what you think it is. Don't forget to assert the proper title before loading anything too.

Done, removed WebKitWebLoaderClient related tests from this patch, and added assert before loading anything for NULL title.

Please note that, I added WebKitWebLoaderClient related test intentionally to test both title and loading (and assert if title is not received after LOAD_COMMIT and before LOAD_FINISH).

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:195
>> +    soup_uri_set_port(baseURI, soup_server_get_port(server));
> 
> I'm almost certain you don't need to make a SoupServer here.

I am afraid that I need to use soup_server, since we don't have the webkit_web_view_load_string functions yet. I have a patch for webkit_web_view_load_string function and will submit it for review soon. I submitted this patch first, since test for webkit_web_view_load_string uses webkit_web_view_get_title function.

[WebKit Review Bot]

Attachment 109452 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  The parameter name "web_view" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:304:  webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

(In reply to comment #17)

> > Here you have the same problem but it's worse even becaues the name of the paramter is @webView.  We really need a build step which verifies the sanity of the documentation. I'd like to have zero warnings when generating it.
> 
> Done. 
> 
> Please note that, I tried to keep the documentation same as webkit1 (https://bugs.webkit.org/show_bug.cgi?id=68074#c5). In fact, few of the methods added earlier also refers to @web_view, hence I followed the same. I am in process of writing a patch to generate documentation for webkit2 gtk+ api, I will address rest of such mismatches in that patch.

It's web_view in public api, so I think the documentation should say web_view too.

> >> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:38
> >> +        soup_message_set_status(msg, SOUP_STATUS_NOT_IMPLEMENTED);
> > 
> > Please don't just copy and paste the entire loading test. The test could be quite simple you could do a notify on the title property of WebView, load a simple string and then assert that that the title is what you think it is. Don't forget to assert the proper title before loading anything too.
> 
> Done, removed WebKitWebLoaderClient related tests from this patch, and added assert before loading anything for NULL title.
> 
> Please note that, I added WebKitWebLoaderClient related test intentionally to test both title and loading (and assert if title is not received after LOAD_COMMIT and before LOAD_FINISH).

Then I think you can just connect to title property in testwebview to check we have something when the document is loaded, and use existing loading tests in testloading to check the title is received between commit and finish (I guess this should be documented too)

[Nayan Kumar K]

Created attachment 109456 [details]
title property added

Updated the patch

[WebKit Review Bot]

Attachment 109456 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  The parameter name "web_view" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:304:  webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nayan Kumar K]

Created attachment 109609 [details]
title property added

Changed the documentation of parameters back to @web_view, as gtk-doc uses header files for documenting declarations.

[WebKit Review Bot]

Attachment 109609 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  The parameter name "web_view" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:99:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:306:  webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gustavo Noronha (kov)]

Comment on attachment 109609 [details]
title property added

Attachment 109609 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9938480

[Martin Robinson]

Comment on attachment 109609 [details]
title property added

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

Looking good in general, but we really need to be careful not to bloat our testing code. I think my comments below will prevent you from copying a lot of code from the loading test. In cases where we do need very similar code we should be abstracting out into helper functions and classes.

Remeber that comments in WebKit should be full sentences that end with a period. Sometimes we miss instances of that, but the style guide is pretty clear about it. I think the only exception is if you are labeling some anonymous parameter.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:53
>      WebKitWebContext* context;
>  
> +    /* Loader Client */
>      GRefPtr<WebKitWebLoaderClient> loaderClient;
> +
> +    /* WebKitWebView properties */
> +    CString title;

Apologies. I don't think my comment was very clear upon re-reading it. Essentially I just suggested that you keep properties and client in separate blocks like:

WebKitWebContext* context;
CString title;
...other properties here later...

I think you can omit the comments altogether.

GRefPtr<WebKitWebLoaderClient> loaderClient;
...other clients here later...

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:59
> +    _WebKitWebViewPrivate()
> +    {
> +        /* Initialize the tile to empty string */
> +        title = CString("");
> +    }

In WebKit we always use initializer fields. No need for a comment. I think you can remove this also (see below).

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:143
> +     * Returns: the main frame title of this #WebKitWebView. If the title has not been received yet, it will be an empty string.

Sorry to nitpick again. :) Please check that the length of this line is consistent with the other documentation. Also, I'm pretty sure that properties don't use the "Returns" keyword. You should double-check with other GTK+ doc though.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:149
> +                                                        _("Returns the main frame title of #WebKitWebView"),

I'd rephrase to say "Main frame document title"

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:151
> +                                                        "",
> +                                                        static_cast<GParamFlags>(WEBKIT_PARAM_READABLE)));

Can't you add G_PARAM_CONSTRUCT_ONLY and avoid haivng a _WebKitWebViewPrivate constuctor above?

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:154
> +void webkitWebViewSetTitle(WebKitWebView *webView, WKStringRef titleRef)

You asterisk is in the wrong place here. Please use const char for the title and convert in the caller.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:304
> + * Returns the property #Object:title.
> + *
> + * Returns: the main frame title of @web_view.

Hrm. It's a bit funky to refer to the property here.  I think what we want to do is if the text is just a little blurb, reproduce it in both the property and getter and setter. If there's a longer description, have it in the getter. In the property documentation we probably just want to have a link that says "See webkit_web_view_get_title"

I don't think you need to have a line that says "Returns" twice.  Please say "the main frame document title..."

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:31
> +#define HTML_TITLE "WelCome to WebKit-GTK+"
> +/* This string has to be rather big because of the cancelled test - it
> + * looks like soup refuses to send or receive a too small chunk */

This comment indicates that you don't need the large string below...there is no cancelled test here.  We don't need the huge HTML string. Additionally, I really want these strings not be defined as preprocessor constants. In WebKit we always prefer using static const char* for things like this. They should also be defined in the tightest scope possible.

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:48
> +/* For real request testing */
> +static void serverCallback(SoupServer* server, SoupMessage* msg, const char* path, GHashTable* query, SoupClientContext* context, gpointer data)
> +{
> +    if (msg->method != SOUP_METHOD_GET) {
> +        soup_message_set_status(msg, SOUP_STATUS_NOT_IMPLEMENTED);
> +        return;
> +    }
> +
> +    soup_message_set_status(msg, SOUP_STATUS_OK);
> +
> +    if (g_str_equal(path, "/"))
> +        soup_message_body_append(msg->response_body, SOUP_MEMORY_STATIC, HTML_STRING, strlen(HTML_STRING));
> +
> +    soup_message_body_complete(msg->response_body);
> +}

Instead of doing this, let's wait until the load_html patch lands and avoid having to use SoupServer.

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:101
> +    /* Assert if title is not empty */
> +    g_assert_cmpstr(webkit_web_view_get_title(fixture->webView), ==, "");

I think you can just omit the comment here.  If you feel the code is not explicit enough, feel free to be more explicit at the expense of redudancy. For instance:

g_assert_cmpstr(webkit_web_view_get_title(fixture->webView), !=, 0);
g_assert_cmpstr(webkit_web_view_get_title(fixture->webView), ==, "");

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:110
> +    /* Assert if title is not received */
> +    g_assert(fixture->hasReceivedTitle);

Instead of storing this information in the fixture, why not just assert that the title is correct here.

[Carlos Garcia Campos]

(In reply to comment #25)
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:151
> > +                                                        "",
> > +                                                        static_cast<GParamFlags>(WEBKIT_PARAM_READABLE)));
> 
> Can't you add G_PARAM_CONSTRUCT_ONLY and avoid haivng a _WebKitWebViewPrivate constuctor above?

It should be CONSTRUCT, not CONSTRUCT_ONLY, but that would be a workaround to avoid initializing the attribute. The title can't be a constructor property because it's not writable, and you are not supposed to be able to create a view with a title. Properties with no value should not return a "", but NULL.

[Carlos Garcia Campos]

Comment on attachment 109609 [details]
title property added

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

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:157
> +    CString title = WebKit::toImpl(titleRef)->string().utf8();

Use const CString& to avoid unnecessary string duplication.

[Martin Robinson]

Comment on attachment 109609 [details]
title property added

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

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:157
>> +    CString title = WebKit::toImpl(titleRef)->string().utf8();
> 
> Use const CString& to avoid unnecessary string duplication.

Hrm. I think that string() is a temporary so this might lead to a reference to a deleted object.

[Nayan Kumar K]

Created attachment 109980 [details]
Added 'title' property

Review comments incorporated.

[WebKit Review Bot]

Attachment 109980 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:106:  The parameter name "web_view" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:106:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:327:  webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nayan Kumar K]

Comment on attachment 109609 [details]
title property added

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

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:53
>> +    CString title;
> 
> Apologies. I don't think my comment was very clear upon re-reading it. Essentially I just suggested that you keep properties and client in separate blocks like:
> 
> WebKitWebContext* context;
> CString title;
> ...other properties here later...
> 
> I think you can omit the comments altogether.
> 
> GRefPtr<WebKitWebLoaderClient> loaderClient;
> ...other clients here later...

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:143
>> +     * Returns: the main frame title of this #WebKitWebView. If the title has not been received yet, it will be an empty string.
> 
> Sorry to nitpick again. :) Please check that the length of this line is consistent with the other documentation. Also, I'm pretty sure that properties don't use the "Returns" keyword. You should double-check with other GTK+ doc though.

Yes, gtk-doc does't have Return keyword for properties. Changed it new patch.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:149
>> +                                                        _("Returns the main frame title of #WebKitWebView"),
> 
> I'd rephrase to say "Main frame document title"

Ok.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:151
>> +                                                        static_cast<GParamFlags>(WEBKIT_PARAM_READABLE)));
> 
> Can't you add G_PARAM_CONSTRUCT_ONLY and avoid haivng a _WebKitWebViewPrivate constuctor above?

Since the property is readable only, we do not have webkit_web_view_set_function and hence using G_PARAM_CONSTRUCT will not be useful.

Moreover, in the last uploaded patch, I am returning 0 if the title has not be received. This is how webkit1 works and I (and Carlos) think we should be consistent with webkit1 API's. Hence we do not need to have the initialization.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:154
>> +void webkitWebViewSetTitle(WebKitWebView *webView, WKStringRef titleRef)
> 
> You asterisk is in the wrong place here. Please use const char for the title and convert in the caller.

If we change the argument to const char*, then we need to cache WebKit::toImpl(titleRef)->string().utf8().data(), send this as argument of webKitWebViewSetTitle and convert it back to CString in webkitWebViewSetTitle implementation, as we copy this variable to WebView property 'title', which is a CString. Since this is a private method, I believe cleaner way to do this is via WKStringRef. (Comment no #17)

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:304
>> + * Returns: the main frame title of @web_view.
> 
> Hrm. It's a bit funky to refer to the property here.  I think what we want to do is if the text is just a little blurb, reproduce it in both the property and getter and setter. If there's a longer description, have it in the getter. In the property documentation we probably just want to have a link that says "See webkit_web_view_get_title"
> 
> I don't think you need to have a line that says "Returns" twice.  Please say "the main frame document title..."

Ok.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:31
>> + * looks like soup refuses to send or receive a too small chunk */
> 
> This comment indicates that you don't need the large string below...there is no cancelled test here.  We don't need the huge HTML string. Additionally, I really want these strings not be defined as preprocessor constants. In WebKit we always prefer using static const char* for things like this. They should also be defined in the tightest scope possible.

I have reduced the length of HTML_STRING in latest uploaded patch. However, I still use these as preprocessor macros, as I need them in 2 different functions. I guess, it makes more sense to make it as preprocessor macro rather than using it as 'static const char *' in two different functions for this test.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:48
>> +}
> 
> Instead of doing this, let's wait until the load_html patch lands and avoid having to use SoupServer.

Done. latest patch uses webkit_web_view_load_html and compares the title received.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:101
>> +    g_assert_cmpstr(webkit_web_view_get_title(fixture->webView), ==, "");
> 
> I think you can just omit the comment here.  If you feel the code is not explicit enough, feel free to be more explicit at the expense of redudancy. For instance:
> 
> g_assert_cmpstr(webkit_web_view_get_title(fixture->webView), !=, 0);
> g_assert_cmpstr(webkit_web_view_get_title(fixture->webView), ==, "");

Removed the comment.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:110
>> +    g_assert(fixture->hasReceivedTitle);
> 
> Instead of storing this information in the fixture, why not just assert that the title is correct here.

Done.

[Martin Robinson]

(In reply to comment #31)
=
> >> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:154
> >> +void webkitWebViewSetTitle(WebKitWebView *webView, WKStringRef titleRef)
> > 
> > You asterisk is in the wrong place here. Please use const char for the title and convert in the caller.
> 
> If we change the argument to const char*, then we need to cache WebKit::toImpl(titleRef)->string().utf8().data(), send this as argument of webKitWebViewSetTitle and convert it back to CString in webkitWebViewSetTitle implementation, as we copy this variable to WebView property 'title', which is a CString. Since this is a private method, I believe cleaner way to do this is via WKStringRef. (Comment no #17)

The issue with using WKStringRef here means that if we want to set the title in another place in the future we need to convert to a WKStringRef. I think the fact that the title was originally a WKStringRef is an implementation detail of the caller. If you are concerned about conversion from CString to const char and then back to CString feel free to pass a "const CString&". I do not prefer using WKStringRef here.

> 
> >> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:304
> >> + * Returns: the main frame title of @web_view.
> > 
> > Hrm. It's a bit funky to refer to the property here.  I think what we want to do is if the text is just a little blurb, reproduce it in both the property and getter and setter. If there's a longer description, have it in the getter. In the property documentation we probably just want to have a link that says "See webkit_web_view_get_title"
> > 
> > I don't think you need to have a line that says "Returns" twice.  Please say "the main frame document title..."
> 
> Ok.
> 
> >> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:31
> >> + * looks like soup refuses to send or receive a too small chunk */
> > 
> > This comment indicates that you don't need the large string below...there is no cancelled test here.  We don't need the huge HTML string. Additionally, I really want these strings not be defined as preprocessor constants. In WebKit we always prefer using static const char* for things like this. They should also be defined in the tightest scope possible.
> 
> I have reduced the length of HTML_STRING in latest uploaded patch. However, I still use these as preprocessor macros, as I need them in 2 different functions. I guess, it makes more sense to make it as preprocessor macro rather than using it as 'static const char *' in two different functions for this test.

The biggest issue is that in WebKit we do not use #define, we use static const char*. If you like you can leave it at the global scope.  Another option is to have it in main and pass it as the data argument to the tests.

I'm going to try to land a unit test overhaul soon, by the way:  https://bugs.webkit.org/show_bug.cgi?id=69409

[Nayan Kumar K]

Created attachment 109993 [details]
title property added

Updated as per review comments.

[WebKit Review Bot]

Attachment 109993 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:106:  The parameter name "web_view" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:106:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:326:  webkit_web_view_get_title is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gustavo Noronha (kov)]

Comment on attachment 109993 [details]
title property added

Attachment 109993 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9978344

[Carlos Garcia Campos]

Comment on attachment 109993 [details]
title property added

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

This doesn't build, I wonder why it applies, because of the changes in current git master.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:144
> +                                                        static_cast<GParamFlags>(WEBKIT_PARAM_READABLE)));

WEBKIT_PARAM_READABLE si already casted, you don't need another cast here.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:363
> + * The main frame document title. If the title has not been received yet,
> + * then NULL will be returned.
> + *
> + * Returns: The main frame title of @web_view.

I prefer something like this:

* Gets the value of #WebKitWebView:title.
* You can connect to ::notify signal of @web_view to 
* be notified when the title has been received.
*
* Returns: The main frame document title of @web_view, or %NULL if the title has not been received yet.

Or something like that. This way you link to the property documentation and user knows that the property can be watched to be notified title changes.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:114
> +WK_EXPORT const gchar * 

Use WEBKIT_API instead WK_EXPORT, just update to current git master and rebase.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewPrivate.h:31
> +#include "WebKitWebView.h"
> +

Don't you need to include wtf/text/CString.h here for CString?

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:37
> +typedef struct {
> +    WebKitWebView *webView;
> +    GMainLoop *loop;
> +} WebViewTitleFixture;

Rename it to somethjing more generic, WEbViewFixture, so that it can be used by other tests too.

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:42
> +    const gchar *title = webkit_web_view_get_title(webView);
> +    g_assert_cmpstr(title, ==, "Welcome to WebKitGtk+!");

This could be just g_assert_cmpstr(webkit_web_view_get_title(webView), ==, "Welcome to WebKitGtk+!");

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:50
> +    g_signal_connect(fixture->webView, "notify::title", G_CALLBACK(webViewNotifyTitle), fixture);

Connect to the signal in  testWebViewTitle since the fixture might be used for other tests.

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:57
> +    g_assert_cmpstr(webkit_web_view_get_title(fixture->webView), ==, 0);

To check it's null just g_assert(!webkit_web_view_get_title(fixture->webView));

[Martin Robinson]

Comment on attachment 109993 [details]
title property added

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

Looking good!

> Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:126
> +static void didReceiveTitleForFrame(WKPageRef page, WKStringRef titleRef, WKFrameRef frameRef, WKTypeRef userData, const void* clientInfo)

userData is unused, so you should omit the name of the parameter.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:135
> +     * The main frame title of this #WebKitWebView. If the title 

You should also say "main frame document" here.

[Carlos Garcia Campos]

Comment on attachment 109993 [details]
title property added

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

> Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:131
> +    const CString title = WebKit::toImpl(titleRef)->string().utf8();

I think you don't need WebKit:: there.

[Nayan Kumar K]

Created attachment 110108 [details]
title property added

[Nayan Kumar K]

Created attachment 110109 [details]
title property added

[Nayan Kumar K]

Created attachment 110110 [details]
title property added

[Collabora GTK+ EWS bot]

Comment on attachment 110110 [details]
title property added

Attachment 110110 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10000152

[Nayan Kumar K]

(In reply to comment #42)
> (From update of attachment 110110 [details])
> Attachment 110110 [details] did not pass gtk-ews (gtk):
> Output: http://queues.webkit.org/results/10000152

This patch depends on https://bugs.webkit.org/show_bug.cgi?id=69249 which introduces webkit_web_view_load_html API.

[Carlos Garcia Campos]

Comment on attachment 110110 [details]
title property added

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

This patch needs to be updated to use the new unit tests system, and it will need to be updated when bug #69814 is fixed too.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:147
> +    const CString title = toImpl(titleRef)->string().utf8();
> +    webkitWebViewSetTitle(WEBKIT_WEB_LOADER_CLIENT(clientInfo)->priv->view.get(), title);

I think we can just avoid caching title here and use a single line

webkitWebViewSetTitle(WEBKIT_WEB_LOADER_CLIENT(clientInfo)->priv->view.get(), toImpl(titleRef)->string().utf8());

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:136
> +     * the title has not been received yet, it will be NULL.

Use %NULL.

[Nayan Kumar K]

Created attachment 111769 [details]
title property added

[WebKit Review Bot]

Comment on attachment 111769 [details]
title property added

Attachment 111769 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10182421

New failing tests:
http/tests/inspector/resource-tree/resource-tree-document-url.html

[Collabora GTK+ EWS bot]

Comment on attachment 111769 [details]
title property added

Attachment 111769 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10179468

[Martin Robinson]

Any clue why this broke the build?

[Martin Robinson]

Comment on attachment 111769 [details]
title property added

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

> Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:126
> +static void didReceiveTitleForFrame(WKPageRef page, WKStringRef titleRef, WKFrameRef frameRef, WKTypeRef, const void* clientInfo)

You can remove the name of clientInfo, since it's unused.

[Nayan Kumar K]

Created attachment 111914 [details]
title property added

Incorporated review comments.

[Gustavo Noronha (kov)]

Comment on attachment 111914 [details]
title property added

Attachment 111914 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10176816

[Carlos Garcia Campos]

Comment on attachment 111914 [details]
title property added

This is already r+, but it still needs some things before landing:
 - Make sure it still applies to current git master, I think it doesn't
 - Add new symbols to gtk-doc sections file
 - Update the unit test to use WebViewTest::LoadHtml() instead of webkit_web_view_load_html() directly

[Nayan Kumar K]

Created attachment 113468 [details]
Add 'title' property

Changes included in this patch are,
a). Rebased the patch to latest webkit trunk.
b). Use WebViewTest::loadHtml instead of webkit_web_view_load_html.
c). Added 'webkit_web_view_get_title' in webkit2gtk-sections.txt file.

[WebKit Review Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Carlos Garcia Campos]

Comment on attachment 113468 [details]
Add 'title' property

LGTM

[Philippe Normand]

Comment on attachment 113468 [details]
Add 'title' property

Thanks!

[WebKit Review Bot]

Comment on attachment 113468 [details]
Add 'title' property

Clearing flags on attachment: 113468

Committed r99179: <http://trac.webkit.org/changeset/99179>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.