Bug 95697 – [WebKit2] [GTK] Add API for controlling the user agent

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Martin Robinson]

Choosing a default user agent and changing it is essential for a proper web engine embedding experience. This bug tracks adding that API to WebKit2GTK+.

[Martin Robinson]

Created attachment 161944 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 161944 [details]
Patch

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

I think this patch could be split in two, one to move the code to webcore and update WebKit1, and another one adding wk2 api. Regarding the wk2 patch, the logic to set user agent and application name is duplicated, because it's already in WebPageProxy. I think we could have two different properties: user-agent and application-name-for-user-agent. The view connects to notify signal for both properties and simply calls WKPageSetCustomUserAgent() for user-agent and WKPageSetApplicationNameForUserAgent() for application-name-for-user-agent.

> Source/WebKit2/ChangeLog:11
> +        in the lirary.

lirary -> library

> Source/WebCore/platform/gtk/UserAgentGtk.cpp:62
> +    uaOSVersion =  "Intel Mac OS X";

Extra space here after =

> Source/WebCore/platform/gtk/UserAgentGtk.cpp:64
> +    uaOSVersion =  "PPC Mac OS X";

Ditto.

> Source/WebCore/platform/gtk/UserAgentGtk.cpp:94
> +    return String::format("%s %s/%s", staticUA.utf8().data(),
> +                          applicationName.isEmpty() ? "WebKitGTK+" : applicationName.utf8().data(), uaVersion.data());

We could use g_get_application_name() or g_get_prgname() since I guess we don't want the localized name here, and maybe fallback to WebKitGTK+ if g_get_prgname() returns NULL.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:51
> +    CString userAgentString;

userAgentString sounds a bit redundant, it's a CString, maybe just userAgent?

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1023
> +     * WebKitSettings:user-agent

Trailing : missing here.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1031
> +     * If this property is set to the empty string or NULL, it will revert to the standard

NULL -> %NULL

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2597
> + * Get the #WebKitSettings::user-agent property.

:: is for signals, for properties use :

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2613
> + * @user_agent: The new custom user agent string

Add (allow-none) annotation. And you should comment here that when passing a NULL user agent, sets the default one.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2615
> + * Set the #WebKitSettings::user-agent property.

#WebKitSettings::user-agent -> #WebKitSettings:user-agent

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2622
> +    CString newUserAgent = (!userAgent || !strlen(userAgent)) ? WebCore::standardUserAgent("").utf8() : userAgent;

(userAgent && userAgent[0] != '\0') ? userAgent : WebCore::standardUserAgent("").utf8()

To avoid using strlen().

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2627
> +    priv->userAgentString = newUserAgent;
> +    g_object_notify(G_OBJECT(settings), "user-agent");

This doesn't actually change the user agent, you should call WKPageSetCustomUserAgent() with the new user agent, so the web view should connect to notify::user-agent to update its user agent when the setting changes.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2633
> + * @application_name: The application name used to create the user agent.

I don't think we should allow NULL for the application name here

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2635
> + * Set the #WebKitSettings::user-agent property by forming the standard user

#WebKitSettings::user-agent -> #WebKitSettings:user-agent

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2637
> + * string or null, this method sets the user agent property to the standard user

null -> %NULL

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2638
> + * agent string with the application name "WebKitGTK+."

That's already the default user agent when this method is not called, so I think we should enforce a non null value for the application name to actually change the user agent based on the application name.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2641
> +void webkit_settings_set_user_agent_with_application_name(WebKitSettings* settings, const char* applicationName)

webkit_settings_set_default_user_agent_with_application_name()?

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2651
> +    WebKitSettingsPrivate* priv = settings->priv;
> +    CString newUserAgent = WebCore::standardUserAgent(String::fromUTF8(applicationName)).utf8();
> +    if (newUserAgent == priv->userAgentString)
> +        return;
> +
> +    priv->userAgentString = newUserAgent;
> +    g_object_notify(G_OBJECT(settings), "user-agent");

We could have a helper function to share this code with webkit_settings_set_user_agent(). 

set_user_agent (WebKitSettings* settings, const char* userAgent, const char* applicationName)

webkit_settings_set_user_agent would call set_user_agent(settings, userAgent, 0)
and webkit_settings_set_default_user_agent_with_application_name would call set_user_agent(settings, 0, applicationName);

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:264
> +    g_assert(g_strstr_len(defaultUserAgent.data(), -1, "Safari"));
> +    g_assert(g_strstr_len(defaultUserAgent.data(), -1, "Chromium"));
> +    g_assert(g_strstr_len(defaultUserAgent.data(), -1, "Chrome"));

Why not passing defaultUserAgent.length() instead of -1

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:268
> +    const char* funkyUserAgent = "Funky!";
> +    webkit_settings_set_user_agent(settings.get(), funkyUserAgent);
> +    g_assert_cmpstr(funkyUserAgent, ==, webkit_settings_get_user_agent(settings.get()));

It would be nice to test also that setting a NULL user agent returns the default user agent

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:277
> +    g_assert_cmpint(strlen(webCatUserAgent.data()), ==, strlen(defaultUserAgent.data()));

could you use .length() instead of strlen?

[Martin Robinson]

Comment on attachment 161944 [details]
Patch

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

>> Source/WebCore/platform/gtk/UserAgentGtk.cpp:94
>> +                          applicationName.isEmpty() ? "WebKitGTK+" : applicationName.utf8().data(), uaVersion.data());
> 
> We could use g_get_application_name() or g_get_prgname() since I guess we don't want the localized name here, and maybe fallback to WebKitGTK+ if g_get_prgname() returns NULL.

That's reasonable, but it could potentially lead to unpredictable interaction with the page. For instance if an application has an unusual name like "Foo / Whatever" it could break the way a page parses the user agent. Perhaps it's better to choose the safest default?

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:51
>> +    CString userAgentString;
> 
> userAgentString sounds a bit redundant, it's a CString, maybe just userAgent?

Sure.

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2622
>> +    CString newUserAgent = (!userAgent || !strlen(userAgent)) ? WebCore::standardUserAgent("").utf8() : userAgent;
> 
> (userAgent && userAgent[0] != '\0') ? userAgent : WebCore::standardUserAgent("").utf8()
> 
> To avoid using strlen().

Is it useful to avoid strlen here? It feels like a micro-optimization.

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2627
>> +    g_object_notify(G_OBJECT(settings), "user-agent");
> 
> This doesn't actually change the user agent, you should call WKPageSetCustomUserAgent() with the new user agent, so the web view should connect to notify::user-agent to update its user agent when the setting changes.

I agree that would be better, but from what I can tell WebKitSettings does not store a page to interact with. Perhaps I was wrong?

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2633
>> + * @application_name: The application name used to create the user agent.
> 
> I don't think we should allow NULL for the application name here

Why? Wouldn't it be better to follow the Rubstness Principle here (http://en.wikipedia.org/wiki/Robustness_principle)?

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2638
>> + * agent string with the application name "WebKitGTK+."
> 
> That's already the default user agent when this method is not called, so I think we should enforce a non null value for the application name to actually change the user agent based on the application name.

I think it makes the APIs nicely symmetric.

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2641
>> +void webkit_settings_set_user_agent_with_application_name(WebKitSettings* settings, const char* applicationName)
> 
> webkit_settings_set_default_user_agent_with_application_name()?

Hrm. Not sure. It isn't the default user agent, it's just like the default user agent.

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2651
>> +    g_object_notify(G_OBJECT(settings), "user-agent");
> 
> We could have a helper function to share this code with webkit_settings_set_user_agent(). 
> 
> set_user_agent (WebKitSettings* settings, const char* userAgent, const char* applicationName)
> 
> webkit_settings_set_user_agent would call set_user_agent(settings, userAgent, 0)
> and webkit_settings_set_default_user_agent_with_application_name would call set_user_agent(settings, 0, applicationName);

Sure.

>> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:264
>> +    g_assert(g_strstr_len(defaultUserAgent.data(), -1, "Chrome"));
> 
> Why not passing defaultUserAgent.length() instead of -1

Well, I suppose this tests that webkit_settings_get_user_agent is returning null terminated strings, but I could easily turn your question around. :)

>> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:268
>> +    g_assert_cmpstr(funkyUserAgent, ==, webkit_settings_get_user_agent(settings.get()));
> 
> It would be nice to test also that setting a NULL user agent returns the default user agent

Good point

>> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:277
>> +    g_assert_cmpint(strlen(webCatUserAgent.data()), ==, strlen(defaultUserAgent.data()));
> 
> could you use .length() instead of strlen?

Yep!

[Carlos Garcia Campos]

(In reply to comment #3)
> (From update of attachment 161944 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=161944&action=review
> 
> >> Source/WebCore/platform/gtk/UserAgentGtk.cpp:94
> >> +                          applicationName.isEmpty() ? "WebKitGTK+" : applicationName.utf8().data(), uaVersion.data());
> > 
> > We could use g_get_application_name() or g_get_prgname() since I guess we don't want the localized name here, and maybe fallback to WebKitGTK+ if g_get_prgname() returns NULL.
> 
> That's reasonable, but it could potentially lead to unpredictable interaction with the page. For instance if an application has an unusual name like "Foo / Whatever" it could break the way a page parses the user agent. Perhaps it's better to choose the safest default?

That's why I said we don't want the localized app name as returned by g_get_application_name(), but the program name returned by g_get_prgname(). 

> >> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2627
> >> +    g_object_notify(G_OBJECT(settings), "user-agent");
> > 
> > This doesn't actually change the user agent, you should call WKPageSetCustomUserAgent() with the new user agent, so the web view should connect to notify::user-agent to update its user agent when the setting changes.
> 
> I agree that would be better, but from what I can tell WebKitSettings does not store a page to interact with. Perhaps I was wrong?

"so the web view should connect to notify::user-agent to update its user agent when the setting changes."

I first reviewed, the code and then made the final comment, most of the comments ae no longer needed if we follow the approach I suggested in the global review comment:

"I think this patch could be split in two, one to move the code to webcore and update WebKit1, and another one adding wk2 api. Regarding the wk2 patch, the logic to set user agent and application name is duplicated, because it's already in WebPageProxy. I think we could have two different properties: user-agent and application-name-for-user-agent. The view connects to notify signal for both properties and simply calls WKPageSetCustomUserAgent() for user-agent and WKPageSetApplicationNameForUserAgent() for application-name-for-user-agent."

So, I propose to use two different properties, so the API code would be simpler, because all the logic is already in WebPageProxy. And I think the API is less confusing, you have user-agent and application-name-for-user-agent. What do you think?

[Carlos Garcia Campos]

(In reply to comment #3)
> (From update of attachment 161944 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=161944&action=review
> 

> >> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:264
> >> +    g_assert(g_strstr_len(defaultUserAgent.data(), -1, "Chrome"));
> > 
> > Why not passing defaultUserAgent.length() instead of -1
> 
> Well, I suppose this tests that webkit_settings_get_user_agent is returning null terminated strings, but I could easily turn your question around. :)

CString uses strlen() when size is not passed to constructor.
 
> >> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:268
> >> +    g_assert_cmpstr(funkyUserAgent, ==, webkit_settings_get_user_agent(settings.get()));
> > 
> > It would be nice to test also that setting a NULL user agent returns the default user agent
> 
> Good point
> 
> >> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:277
> >> +    g_assert_cmpint(strlen(webCatUserAgent.data()), ==, strlen(defaultUserAgent.data()));
> > 
> > could you use .length() instead of strlen?
> 
> Yep!

[Martin Robinson]

(In reply to comment #2)
> (From update of attachment 161944 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=161944&action=review
> 
> I think this patch could be split in two, one to move the code to webcore and update WebKit1, and another one adding wk2 api. Regarding the wk2 patch, the logic to set user agent and application name is duplicated, because it's already in WebPageProxy. I think we could have two different properties: user-agent and application-name-for-user-agent. The view connects to notify signal for both properties and simply calls WKPageSetCustomUserAgent() for user-agent and WKPageSetApplicationNameForUserAgent() for application-name-for-user-agent.

On splitting the patch: Typically the barrier for splitting patches for the same bug/feature is whether they can be reviewed easily. I can split this, but are you sure it's totally necessary here? The code movement to WebCore is small and doesn't change the structure of the original code (it's still three free-floating functions). I didn't turn anything into a class or radically alter it. The only change is that the application name is now part of the user agent string.

On code duplication:  The version in WebPageProxy is for the WebKit2 C API, while the version in the majority of the code for this patch is for the GObject API.

On having two properties: While I considered having two properties initially, given the fact that we don't control the page directly, the interface presented here is simpler, and it actually reflects what the C API does behind the scenes (just changes the user agent property in WebCore), I opted not to go that route.

In addition to  adding another property, introducing another property name also makes the logic of the patch more complicated. What do you do when both are set, for instance? One has to take precedence over the other and this must be documented. It introduces the same questions into the mind of the user of the API. This is what I mean by the interface being simpler. I ultimately chose to design it in a way where I didn't have to answer these questions. 

The properties are actually not independent of one another. If you examine the WebKit C API implementation you'll find that while WebPageProxy stores both the custom user agent and the user agent via the application name, changing these properties just changes the the same setting in WebCore, the user agent.

It's also important to consider that we cannot call WKPageSetCustomUserAgent when the property changes. To call WKPageSetCustomUserAgent requires having a page and currently WebKitSettings does not store pages to which it has been attached -- though I think that would be a noble effort for the future.

[Carlos Garcia Campos]

(In reply to comment #6)
> (In reply to comment #2)
> > (From update of attachment 161944 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=161944&action=review
> > 
> > I think this patch could be split in two, one to move the code to webcore and update WebKit1, and another one adding wk2 api. Regarding the wk2 patch, the logic to set user agent and application name is duplicated, because it's already in WebPageProxy. I think we could have two different properties: user-agent and application-name-for-user-agent. The view connects to notify signal for both properties and simply calls WKPageSetCustomUserAgent() for user-agent and WKPageSetApplicationNameForUserAgent() for application-name-for-user-agent.
> 
> On splitting the patch: Typically the barrier for splitting patches for the same bug/feature is whether they can be reviewed easily. I can split this, but are you sure it's totally necessary here? The code movement to WebCore is small and doesn't change the structure of the original code (it's still three free-floating functions). I didn't turn anything into a class or radically alter it. The only change is that the application name is now part of the user agent string.

The bug is marked as [WebKit2] but it changes WebCore and WebKit1 code, I still think it would be better to split it.

> On code duplication:  The version in WebPageProxy is for the WebKit2 C API, while the version in the majority of the code for this patch is for the GObject API.

I know, I meant you avoid that duplication using the two properties.

> On having two properties: While I considered having two properties initially, given the fact that we don't control the page directly, the interface presented here is simpler, and it actually reflects what the C API does behind the scenes (just changes the user agent property in WebCore), I opted not to go that route.

It makes sense. What confused me when I read the patch was webkit_settings_set_user_agent_with_application_name(), because it doesn't receive a user agent to set. It sets the application name to be used when the default user agent is used. 

> In addition to  adding another property, introducing another property name also makes the logic of the patch more complicated. What do you do when both are set, for instance? One has to take precedence over the other and this must be documented. It introduces the same questions into the mind of the user of the API. This is what I mean by the interface being simpler. I ultimately chose to design it in a way where I didn't have to answer these questions. 

I see your point, in your patch it's clear that both methods change the user agent.

> The properties are actually not independent of one another. If you examine the WebKit C API implementation you'll find that while WebPageProxy stores both the custom user agent and the user agent via the application name, changing these properties just changes the the same setting in WebCore, the user agent.

Yeah.

> It's also important to consider that we cannot call WKPageSetCustomUserAgent when the property changes. To call WKPageSetCustomUserAgent requires having a page and currently WebKitSettings does not store pages to which it has been attached -- though I think that would be a noble effort for the future.

Again, the web view should connect to the notify signal of its settings object to update the WebPage user agent when the property changes.

[Martin Robinson]

Created attachment 162588 [details]
Patch

[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 162588 [details]
Patch

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

As I said this is not enough, if WKPageSetCustomUserAgent() is not called when the user agent property changes, the new user agent won't be sent to the WebProcess and it won't have any effect. See how it's done for zoom-text-only and allow-modal-dialogs properties in webkitWebViewSetSettings(). You should connect to the notify of user-agent from the view and call WKPageSetCustomUserAgent() from the callback using the new user agent string. You should also check that this is actually happening in the unit tests. See languages test in TestWebKitWebContext, you could use a soup server and get the header "User-Agent" after doing any request to check it's what you want.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1085
> +    WKRetainPtr<WKStringRef> userAgent= adoptWK(WKStringCreateWithUTF8CString(priv->userAgent.data()));

userAgent= -> userAgent =

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2636
> + * @application_name: The application name used to create the user agent. If the value passed
> + * is %NULL or the empty string, the default user agent will be used.
> + * @application_version: The application version used to create the user agent. If the value passed
> + * is %NULL or the empty string, the version of Safari at the time of this release will be used.

Leave a single line, and add the explanation in the body.

@application_name: (allow-none): The application name used to create the user agent.
@application_version: (allow-none): The application version used to create the user agent.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2642
> + */
> +
> +void webkit_settings_set_user_agent_with_application_details(WebKitSettings* settings, const char* applicationName, const char* applicationVersion)

extra line there

[Martin Robinson]

(In reply to comment #10)
> (From update of attachment 162588 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162588&action=review
> 
> As I said this is not enough, if WKPageSetCustomUserAgent() is not called when the user agent property changes, the new user agent won't be sent to the WebProcess and it won't have any effect. See how it's done for zoom-text-only and allow-modal-dialogs properties in webkitWebViewSetSettings(). You should connect to the notify of user-agent from the view and call WKPageSetCustomUserAgent() from the callback using the new user agent string. You should also check that this is actually happening in the unit tests. See languages test in TestWebKitWebContext, you could use a soup server and get the header "User-Agent" after doing any request to check it's what you want.

Ah, I understand what you are saying now. Thanks. I've done this in my updated patch.

> > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1085
> > +    WKRetainPtr<WKStringRef> userAgent= adoptWK(WKStringCreateWithUTF8CString(priv->userAgent.data()));
> 
> userAgent= -> userAgent =

Fixed.

> > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2636
> > + * @application_name: The application name used to create the user agent. If the value passed
> > + * is %NULL or the empty string, the default user agent will be used.
> > + * @application_version: The application version used to create the user agent. If the value passed
> > + * is %NULL or the empty string, the version of Safari at the time of this release will be used.
> 
> Leave a single line, and add the explanation in the body.

I'm not sure what your reasoning is here, but I've tightened up the language in the argument descriptions. The explanation is still there, but it's not longer than the ones for GtkWidget.

> @application_name: (allow-none): The application name used to create the user agent.
> @application_version: (allow-none): The application version used to create the user agent.
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2642
> > + */
> > +
> > +void webkit_settings_set_user_agent_with_application_details(WebKitSettings* settings, const char* applicationName, const char* applicationVersion)
> 
> extra line there

Removed.

[Martin Robinson]

Created attachment 162840 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 162840 [details]
Patch

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

Looks good to me, please fix the minor issues I mentioned before landing, and consider adding a test case to check the user agent in the HTTP headers.

> Source/WebKit2/ChangeLog:34
> +

It seems changes in WebKitWebView are not mentioned in the ChangeLog.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1025
> +     * The user-agent string used by WebKit in the headers. Unusual user-agent strings

in the HTTP headers?

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2613
> + * @user_agent: (allow-none) The new custom user agent string or %NULL to use the default user agent

: is missing after the annotation

@user_agent: (allow-none): The new custom user agent string or %NULL to use the default user agent

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2634
> + * @application_name: (allow-none) The application name used for the user agent or %NULL to use the default user agent.
> + * @application_version: (allow-none) The application version for the user agent or %NULL to user the default version.

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2638
> + * Set the #WebKitSettings:user-agent property by appending the application details to the default user
> + * agent. If no application name or version is given, the default user agent used will be used. If only
> + * the version is given, the default engine version is used with the given application name.

Yes this is what I meant by moving the explanation to the body, thanks.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:289
> +}
> +

It would be great to include a test case to check that the expected user agent is actually included in the HTTP headers, it could be a simple test similar to the one used for languages.

[Martin Robinson]

(In reply to comment #13)

> > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1025
> > +     * The user-agent string used by WebKit in the headers. Unusual user-agent strings
> 
> in the HTTP headers?

I just removed the mention of the headers. It was a bit awkward there.


> > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:289
> > +}
> > +
> 
> It would be great to include a test case to check that the expected user agent is actually included in the HTTP headers, it could be a simple test similar to the one used for languages.

Included.

Thanks for the review.

[Martin Robinson]

Committed r128960: <http://trac.webkit.org/changeset/128960>

[Thierry Reding]

(In reply to comment #15)
> Committed r128960: <http://trac.webkit.org/changeset/128960>

This breaks compilation for me. UserAgentGtk.h seems to be missing.

[Thierry Reding]

(In reply to comment #16)
> (In reply to comment #15)
> > Committed r128960: <http://trac.webkit.org/changeset/128960>
> 
> This breaks compilation for me. UserAgentGtk.h seems to be missing.

Okay, if cherry-picking specific commits I should really make sure I
pick all the dependencies as well. Sorry for the noise.