As discussed in webkit-dev (https://lists.webkit.org/pipermail/webkit-dev/2011-February/015952.html) all WebKit ports should drop the language tag part from the User Agent string. Patch will follow. I think we should consider bumping up the WebKit version with this change (e.g. MINOR_VERSION in Version.xcconfig). Feedback on this is would be appreciated.
I don't think that changing Version.xccconfig would be right. These versions only really reflect Apple's internal build procedures. The minor version in Version.xccconfig changes pretty frequently anyway.
Is this an appropriate bug to also talk about removing the "U;" for encryption, and, on Windows, the initial "Windows;" that is redundant with the more-specific version identifier afterwards? Removing those as well would match Firefox 4 ( http://hacks.mozilla.org/2010/09/final-user-agent-string-for-firefox-4/ ), and if we want to do them eventually, it might be better to do these all at once. Certainly Chromium would be interested in seeing them removed.
Changes other than dropping language are tracked by a meta bug 54556.
(In reply to comment #3) > Changes other than dropping language are tracked by a meta bug 54556. I see. I filed bugs on these other two items and blocked them against that bug.
<rdar://problem/8983585>
Created attachment 82813 [details] first try
Chrome User Agent string seems to be constructed outside of WebKit tree (user_agent.cc).
Attachment 82813 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/ChangeLog', u'Source/WebKit/..." exit_code: 1 Source/WebKit2/UIProcess/win/WebPageProxyWin.cpp:108: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 82813 [details] first try View in context: https://bugs.webkit.org/attachment.cgi?id=82813&action=review What exactly does this change affect? Is it all of navigator.userAgent, navigator.appVersion, and HTTP User-Agent header field? Can you add tests for all of these? > Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:-124 > - ua += "; "; > - ua += defaultLanguage(); // Localization information There is likely an #include to remove in this file. > Source/WebKit/efl/ewk/ewk_settings.cpp:326 > - WTF::String static_ua = makeString("Mozilla/5.0 (", _ewk_settings_webkit_platform_get(), "; U; ", _ewk_settings_webkit_os_version_get(), "; ", WebCore::defaultLanguage(), ") AppleWebKit/", ua_version) + makeString(" (KHTML, like Gecko) Version/5.0 Safari/", ua_version); > + WTF::String static_ua = makeString("Mozilla/5.0 (", _ewk_settings_webkit_platform_get(), "; U; ", _ewk_settings_webkit_os_version_get(), ") AppleWebKit/", ua_version) + makeString(" (KHTML, like Gecko) Version/5.0 Safari/", ua_version); Ditto. > Source/WebKit/mac/WebView/WebView.mm:-551 > - NSString *language = [NSUserDefaults _webkit_preferredLanguageCode]; Ditto. > Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:-96 > String language = defaultLanguage(); > > if (applicationNameForUserAgent.isEmpty()) > - return makeString("Mozilla/5.0 (Macintosh; U; " PROCESSOR " Mac OS X ", osVersion, "; ", language, ") AppleWebKit/", webKitVersion, " (KHTML, like Gecko)"); > - return makeString("Mozilla/5.0 (Macintosh; U; " PROCESSOR " Mac OS X ", osVersion, "; ", language, ") AppleWebKit/", webKitVersion, " (KHTML, like Gecko) ", applicationNameForUserAgent); This code won't even compile as is, due to unused variable warning.
Created attachment 83080 [details] LayoutTests added Thanks for the review Alexey. Added tests + fixed the review comments.
Comment on attachment 83080 [details] LayoutTests added View in context: https://bugs.webkit.org/attachment.cgi?id=83080&action=review Looks great. I have to say r- because this badly breaks Safari on Mac, but it's really almost like r+. > Source/WebKit/ChangeLog:12 > + * WebKit.xcodeproj/project.pbxproj: Remove WebNSUserDefaultsExtras.h > + and WebNSUserDefaultsExtras.mm from the project. This cannot be done (see below). > Source/WebKit/mac/ChangeLog:11 > + * Misc/WebNSUserDefaultsExtras.h: Removed as it was only used to > + expose defaultLanguage() in the user agent string. > + > + * Misc/WebNSUserDefaultsExtras.mm: Ditto. These cannot be removed, because Safari uses +[NSUserDefaults _webkit_preferredLanguageCode]. I should have warned you about this pitfall. > LayoutTests/http/tests/navigation/useragent.php:9 > + layoutTestController.setPOSIXLocale("en_US.iso88591"); Why? I don't see how POSIX locale would affect the language - and I also don't see why forcing a language would be good for this test. > LayoutTests/http/tests/navigation/useragent.php:34 > + > \ No newline at end of file Please remove the extra spaces that confused Subversion.
Created attachment 83218 [details] address review comments I should have caught the Safari break. Setting POSIX locale was "inspired by" http://trac.webkit.org/browser/trunk/LayoutTests/fast/js/navigator-language.html. You're right that it is not needed, it was an attempt to make the test more predictable.
Comment on attachment 83218 [details] address review comments Looks great. I assume that Qt API changes will be (or have been) vetted as appropriate.
Thanks for the review. There should not be an API change for QtWebKit - other than the fact that the API now returns a different UA string. Is there any other coordination or testing we should do before landing ? Peter suggested that Chromium could get some real-world feedback quickly.
> There should not be an API change for QtWebKit - other than the fact that the API now returns a different UA string. What I meant was that %Language% was now ignored, which contradicts current header documentation. I think that it's fine to just land this.
(In reply to comment #14) > Is there any other coordination or testing we should do before landing ? Peter suggested that Chromium could get some real-world feedback quickly. Once you land this I will land a change to make Chromium's UA string also drop this tag.
Comment on attachment 83218 [details] address review comments Clearing flags on attachment: 83218 Committed r79396: <http://trac.webkit.org/changeset/79396>
All reviewed patches have been landed. Closing bug.