[webkit-reviews] review denied: [Bug 54560] Drop the language tag part from the User Agent string : [Attachment 83080] LayoutTests added

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Feb 19 21:11:01 PST 2011


Alexey Proskuryakov <ap at webkit.org> has denied Laszlo Gombos
<laszlo.1.gombos at nokia.com>'s request for review:
Bug 54560: Drop the language tag part from the User Agent string
https://bugs.webkit.org/show_bug.cgi?id=54560

Attachment 83080: LayoutTests added
https://bugs.webkit.org/attachment.cgi?id=83080&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
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.


More information about the webkit-reviews mailing list