"Dynamic" determination of locale string for UA

RESOLVED FIXED

Status

Camino Graveyard
OS Integration
--
minor
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: Smokey Ardisson (offline for a while; not following bugs - do not email), Assigned: Mark Mentovai)

Tracking

({fixed1.8.1.1})

Details

Attachments

(1 attachment, 2 obsolete attachments)

4.67 KB, patch
Details | Diff | Splinter Review
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.
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

12 years ago
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)?
(Assignee)

Comment 3

12 years ago
Created attachment 216687 [details] [diff] [review]
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".
Assignee: mikepinkerton → mark
Status: NEW → ASSIGNED
Attachment #216687 - Flags: superreview?(mikepinkerton)
Attachment #216687 - Flags: review?(hwaara)
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 ?
(Assignee)

Comment 5

12 years ago
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

12 years ago
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.
(Assignee)

Comment 7

12 years ago
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

12 years ago
Comment on attachment 216687 [details] [diff] [review]
Fix

Looks good then!
Attachment #216687 - Flags: review?(hwaara) → review+
Comment on attachment 216687 [details] [diff] [review]
Fix

sr=pink
Attachment #216687 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Comment 10

12 years ago
Landed, trunk and 1_8 for 1.8.1.  Trunk should fix bug 335026, although 335026 is still a bug in the fallback case.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
This broke the Camino tree on the trunk:

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

Comment 12

12 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

12 years ago
Keywords: fixed1.8.1
(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 ?
(Assignee)

Comment 14

12 years ago
NSLocale is new in 10.4.
Created attachment 219431 [details] [diff] [review]
rought

mento how about doing soemthing like that ?
(Assignee)

Updated

12 years ago
Attachment #219431 - Flags: review?
(Assignee)

Updated

12 years ago
Attachment #219431 - Flags: review? → review?(mark)
Blocks: 336061
Blocks: 335026
OS: Mac OS X 10.2 → Mac OS X 10.3

Comment 16

11 years ago
Bump. Can we get some commentary on Ludo's patch? It'd be nice to have this fixed soon-ish.
(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.
Can we land "Fix" now (or as soon as the SDK switch is both places)?
Depends on: 359218

Comment 19

11 years ago
Created attachment 245033 [details] [diff] [review]
Unbitrotted "Fix"

This is the bitrot police.
Attachment #216687 - Attachment is obsolete: true
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).
Attachment #219431 - Attachment is obsolete: true
Attachment #219431 - Flags: review?(mark)

Comment 21

11 years ago
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 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 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

11 years ago
Checked in on trunk and 1.8branch.
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Keywords: fixed1.8.1.1
You need to log in before you can comment on or make changes to this bug.