331576 - "Dynamic" determination of locale string for UA

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Just like we use the user's OS language preference for the accept-lang header, we really should use the user's OS locale pref to override the en-US setting in general.useragent.locale in the user-agent.  It seems silly for all our builds (particularly the MultiLang ones) to be reporting a locale of en-US (which we get because we don't do Moz l10n and we build the en-US embedding locale) in their UA.

(Sam had some nice stats about jp.cb.o visits...other browsers all had ja-JP locales, but all the Camino visits had en-US locales...)

Presumably this should be very similar to the code Bruce already wrote to set intl.accept_languages. I don't know if sites use the locale rather than the accept-lang for anything, but it seems like we should do the right thing regardless.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This recently showed up in Hendrix (why are we in Hendrix? Did anyone ask us?):

http://groups.google.com/group/mozilla.feedback/browse_thread/thread/2fbedbbce2193e59/ba76a0af7dd204d1?q=%22Product%3A+Camino%22&rnum=2#ba76a0af7dd204d1

Comment 2

[Håkan Waara]

Should a user that gets en-US in one case (the one already fixed) also get it in this case? I.e., are the values equal in form (and meaning)?

Comment 3

[Mark Mentovai]

Attachment: Fix

Note: we ship with an "English" localization, not "U.S. English", so this will make the default UA locale "en" instead of "en-US".

Comment 4

[Ludovic Hirlimann [:Usul]]

Comment on attachment 216687 [details] [diff] [review]
Fix

+#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_3
+// This long-winded function was introduced in 10.3.  That's fine, because
+// our minimum runtime is now 10.3, but we're still using the 10.2 SDK so
+// it must be declared here.


Why aren't we linking against 10.3 Sdks then , is it a tinderbox issue ?

Comment 5

[Mark Mentovai]

It's a 10.3 SDK issue.  That SDK sucks.  We need to be careful when linking against it, or we won't run on 10.4 at all.  Bug 292530, bug 331576 (especially bug 331576 comment 10).  We'll probably be able to switch SDKs soon.

An alternative is to use the 10.4 SDK, which does not suck, but to avoid using APIs that aren't present in 10.3.  This will cause some behavioral differences between running on 10.3 and 10.4.  (Some of those might be differences we'd actually want - the 10.4 SDK enables pixel-granularity scroll wheels when running on 10.4.)

For now, the forward declaration is fine.  We'll be able to clean them all up by grepping for MAC_OS_X_VERSION and friends once we switch SDKs.

Comment 6

[Håkan Waara]

Comment on attachment 216687 [details] [diff] [review]
Fix

>+      // ab or ab-CD form
>+      NSArray* localizations = [[NSBundle mainBundle] preferredLocalizations];

I think we should use the locale that the system is running under. Will this API do that, or only look at the available localization(s) in the bundle? For example, I'd want sv-SE even if I happen to run a english Camino.

Comment 7

[Mark Mentovai]

I don't think you do.  You want sv-SE in accept-lang.  You want the UI locale (en) in the UA string.

Comment 8

[Håkan Waara]

Comment on attachment 216687 [details] [diff] [review]
Fix

Looks good then!

Comment 9

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 216687 [details] [diff] [review]
Fix

sr=pink

Comment 10

[Mark Mentovai]

Landed, trunk and 1_8 for 1.8.1.  Trunk should fix bug 335026, although 335026 is still a bug in the fallback case.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This broke the Camino tree on the trunk:

ld: Undefined symbols:
CFLocaleCreateCanonicalLocaleIdentifierFromString(__CFAllocator const*, __CFString const*)
** BUILD FAILED **

Comment 12

[Mark Mentovai]

Backed out because of flaming tinderboxen.

ld: Undefined symbols:
CFLocaleCreateCanonicalLocaleIdentifierFromString(__CFAllocator const*, __CFString const*)

The function was introduced in the 10.3 SDK and ppc still builds against the 10.2 SDK.  I accounted for that in the C, but there's still nothing to link against.

Comment 13

[Ludovic Hirlimann [:Usul]]

(In reply to comment #12)
 
> The function was introduced in the 10.3 SDK and ppc still builds against the
> 10.2 SDK.  I accounted for that in the C, but there's still nothing to link
> against.
> 

How about following that route : http://www.cocoabuilder.com/archive/message/cocoa/2005/8/18/144585 this works since 9.1 or so ?

Comment 14

[Mark Mentovai]

NSLocale is new in 10.4.

Comment 15

[Ludovic Hirlimann [:Usul]]

Attachment: rought

mento how about doing soemthing like that ?

Comment 16

[Chris Lawson (gone)]

Bump. Can we get some commentary on Ludo's patch? It'd be nice to have this fixed soon-ish.

Comment 17

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #16)
> Bump. Can we get some commentary on Ludo's patch? 

It doesn't seem to work.  After applying the patch and rebuilding, I still have the bogus locale value.

Comment 18

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Can we land "Fix" now (or as soon as the SDK switch is both places)?

Comment 19

[froodian (Ian Leue)]

Attachment: Unbitrotted "Fix"

This is the bitrot police.

Comment 20

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 219431 [details] [diff] [review]
rought

Obsoleting this patch and cancelling the review request since we can land mento's fix on the trunk (and on the branch shortly).

Comment 21

[froodian (Ian Leue)]

Comment on attachment 245033 [details] [diff] [review]
Unbitrotted "Fix"

This patch doesn't seem to work, whether through faulty unbitrotting or something changing underneath it.

Comment 22

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 245033 [details] [diff] [review]
Unbitrotted "Fix"

Did you test this against an English-only Camino, or a multilingual one?  It takes the language from the Camino UI localization in use, not from the system prefs directly.

Regardless, this patch fixes the b0rken UA on the trunk, and I think we should land it to stop all the crashes/bugs on the trunk related to the b0rken UA (i.e., it gets us back to where we were before, an "en" app locale for everyone).

Comment 23

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 245033 [details] [diff] [review]
Unbitrotted "Fix"

No, it works properly.  

I hacked a fake French Camino (cp English.lproj fr.lproj, hacked MainMenu.nib in fr.lproj to show some French words), and ran Camino.  I had French menu titles and this UA: 
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; fr; rv:1.9a1) Gecko/20061112 Camino/1.2+

Comment 24

[froodian (Ian Leue)]

Checked in on trunk and 1.8branch.