We can move user agent helpers to WebCore so that they can more easily be shared with WebKit2.
Created attachment 162584 [details] Patch
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?
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.
(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.
Created attachment 162788 [details] Patch
Comment on attachment 162788 [details] Patch Clearing flags on attachment: 162788 Committed r127889: <http://trac.webkit.org/changeset/127889>
All reviewed patches have been landed. Closing bug.