Bug 95745 – [GTK] Move user agent helpers to WebCore

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Martin Robinson]

We can move user agent helpers to WebCore so that they can more easily be shared with WebKit2.

[Martin Robinson]

Created attachment 162584 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 162584 [details]
Patch

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

Looks good to me, just consider adding a default application name when not provided.

> Source/WebCore/platform/gtk/UserAgentGtk.cpp:102
> +    return String::format("%s %s/%s", staticUA.utf8().data(), applicationName.utf8().data(), finalApplicationVersion.utf8().data());

So, if application name is empty, we end up with something like "/1.2.3", it's a bit weird. If we don't want to use the program name as default we could use something like "WebKitGTK+ based browser/1.2.3"

> Source/WebCore/platform/gtk/UserAgentGtk.h.in:38
> +const char* platformForUAString();
> +String platformVersionForUAString();

Do we need these to be public?

[Martin Robinson]

Comment on attachment 162584 [details]
Patch

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

>> Source/WebCore/platform/gtk/UserAgentGtk.cpp:102
>> +    return String::format("%s %s/%s", staticUA.utf8().data(), applicationName.utf8().data(), finalApplicationVersion.utf8().data());
> 
> So, if application name is empty, we end up with something like "/1.2.3", it's a bit weird. If we don't want to use the program name as default we could use something like "WebKitGTK+ based browser/1.2.3"

If the application name is empty, we've already returned early by this point because at line 95 we have:

if (applicationName.isEmpty())
    return staticUA;

>> Source/WebCore/platform/gtk/UserAgentGtk.h.in:38
>> +String platformVersionForUAString();
> 
> Do we need these to be public?

Nope! They were left over from a previous design. I'll make them static.

[Carlos Garcia Campos]

(In reply to comment #3)
> (From update of attachment 162584 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162584&action=review
> 
> >> Source/WebCore/platform/gtk/UserAgentGtk.cpp:102
> >> +    return String::format("%s %s/%s", staticUA.utf8().data(), applicationName.utf8().data(), finalApplicationVersion.utf8().data());
> > 
> > So, if application name is empty, we end up with something like "/1.2.3", it's a bit weird. If we don't want to use the program name as default we could use something like "WebKitGTK+ based browser/1.2.3"
> 
> If the application name is empty, we've already returned early by this point because at line 95 we have:
> 
> if (applicationName.isEmpty())
>     return staticUA;

Perfect then, I missed that, sorry.

> >> Source/WebCore/platform/gtk/UserAgentGtk.h.in:38
> >> +String platformVersionForUAString();
> > 
> > Do we need these to be public?
> 
> Nope! They were left over from a previous design. I'll make them static.

Ok.

[Martin Robinson]

Created attachment 162788 [details]
Patch

[Martin Robinson]

Comment on attachment 162788 [details]
Patch

Clearing flags on attachment: 162788

Committed r127889: <http://trac.webkit.org/changeset/127889>

[Martin Robinson]

All reviewed patches have been landed.  Closing bug.