Investigate whether we can get country data from mac OS APIs

RESOLVED FIXED in Firefox 35

Status

()

Core
XPCOM
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: Benjamin Smedberg, Assigned: smichaud)

Tracking

(Blocks: 1 bug)

unspecified
mozilla36
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox34- wontfix, firefox35+ fixed, firefox36+ fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
In order to target the new Yahoo! search default, we'd like to be able to change the default of users within the US but not some other countries (particularly asian countries).

Our base plan is to do this by time zone, but we'd like to investigate targeting this a little more carefully by country based on information already contained in the OS. Steven, can you provide an overview what other information we can obtain from MacOS about the user's country, and a basic estimate of the effort required?

This is extremely urgent, if you can't provide info by today please let me know.
Flags: needinfo?(smichaud)
(Reporter)

Updated

3 years ago
Blocks: 1102297
No longer blocks: 1102297
(Assignee)

Comment 1

3 years ago
I *think* we can programmatically get the information entered by the user in the Time Zone panel of the Date & Time system pref panel.  What can be set directly is "Closest City".  The "Time Zone" setting is derived from that.

There are *lots* of cities that can be specified.  But I think the "country" is also always specified.

If the user hasn't changed this information, I believe it defaults to Cupertino, CA.  In the US, at least.  But if the user *doesn't* change this information, their clock will almost certainly be incorrect -- so I think it's likely that most users will have set this information.

With luck, later I'll post a test app that gets (and prints) this information.
(Reporter)

Comment 2

3 years ago
ok, if it's possible to do this as a patch to nsSystemInfo.cpp that would be useful. I'm traveling for the rest of the day, but Gavin can help decide if this is something we want to use in FF34.
(Assignee)

Comment 3

3 years ago
> if it's possible to do this as a patch to nsSystemInfo.cpp

Not possible.  It will need to use Cocoa syntax.
(Assignee)

Comment 4

3 years ago
Created attachment 8526287 [details]
Test app

Here's my test app.  I tested it (and it worked fine) on the two endpoints of our current OS X support -- OS X 10.6.8 and OS X 10.10.1.

I assume what we want is the "CountryCode".

Let me see if I can add code that queries this to Cocoa code, which can then be called from something in nsSystemInfo.cpp.

Gavin, do you have any suggestions about how I should do this?
Flags: needinfo?(gavin.sharp)
(Assignee)

Updated

3 years ago
Attachment #8526287 - Attachment mime type: text/x-troff-mm → text/plain
Flags: needinfo?(smichaud)
You could add a "countryCode" attribute to widget/nsIGfxInfo.idl, and the corresponding implementation to widget/cocoa/GfxInfo.mm. Abuse of a graphics-related API for something that isn't, but I think that's fine (it's kind of a dumping ground already).
Flags: needinfo?(gavin.sharp)
(Assignee)

Comment 6

3 years ago
Created attachment 8526415 [details] [diff] [review]
Fix

I followed Gavin's suggestion.  nsSystemInfo.cpp seems inappropriate, because none of its values are string values.

This works in my tests.  I tested it by opening the Scratchpad in Developer Tools, choosing Browser for the environment, running the following lines, then choosing Display.  The result on my machine was "US" (as expected).

var gfxInfo = Components.classes['@mozilla.org/gfx/info;1'].
              getService(Components.interfaces.nsIGfxInfo);
print(gfxInfo.countryCode);
Attachment #8526415 - Flags: review?(benjamin)
(Assignee)

Comment 7

3 years ago
Comment on attachment 8526415 [details] [diff] [review]
Fix

Actually maybe it's best to ask Gavin to review this, since Benjamin may be unavailable for a while.
Attachment #8526415 - Flags: review?(benjamin) → review?(gavin.sharp)
status-firefox34: --- → affected
status-firefox35: --- → affected
status-firefox36: --- → affected
tracking-firefox34: --- → +
tracking-firefox35: --- → +
tracking-firefox36: --- → +
Comment on attachment 8526415 [details] [diff] [review]
Fix

I think you probably want to use "Append" instead of "AppendASCII" in GfxInfo::GetSelectedCityInfo?

I also think we may want to put the new attribute at the bottom of the IDL definition, and avoid changing the IID.

This looks good to me. We're going to need a very detailed enumeration of the possible values here - I think we care to identify only "US" and "Canada".

This patch will also need no-op implementations for the other platforms (they can just return null, ideally). Is there Apple documentation for that?
Attachment #8526415 - Flags: review?(gavin.sharp)
Attachment #8526415 - Flags: review?(benjamin)
Attachment #8526415 - Flags: feedback+
Blocks: 1102416
(Assignee)

Comment 9

3 years ago
Comment on attachment 8526415 [details] [diff] [review]
Fix

> I think you probably want to use "Append" instead of "AppendASCII"
> in GfxInfo::GetSelectedCityInfo?

Really?  I'd think not.  But I don't understand our (horribly complex)
XPCOM string api very well.  I'll let Benjamin decide.

> I also think we may want to put the new attribute at the bottom of
> the IDL definition, and avoid changing the IID.

Why should we avoid changing the IID?  There's already a note at the
top of nsIGfxInfo.idl which says "this interface is completely
undesigned, not stable and likely to change".  But once again, I'll
let Benjamin decide.

> We're going to need a very detailed enumeration of the possible
> values here

Let me see what I can come up with.

> I think we care to identify only "US" and "Canada"

But if this is the case, getting a comprehensive list of the possible
values is less urgent.  Let me check about "Canada" -- I suspect all
of these country codes are two-letter values (the same ones that are
used for "language" country codes).

> This patch will also need no-op implementations for the other
> platforms

I thought about this but decided not to, at least for the time being.
It'd be trivial to do, though.  And we might actually want to use the
"countryCode" value on other platforms -- if (for example) we can come
up with a way to translate timezone names into country codes.
> Let me check about "Canada"

I just checked, and the "country code" for Canada is "CA".

If someone could give me a list of countries we care about, I can just check Apple's "CountryCode" for each of them (by deliberately choosing the wrong "closest city" in the Date & Time preference panel).
(In reply to Steven Michaud from comment #9)
> Really?  I'd think not.  But I don't understand our (horribly complex)
> XPCOM string api very well.  I'll let Benjamin decide.

Given the variable name I assumed it contained UTF-8 text, so it seems wrong to treat it as ASCII. But it doesn't really matter if all possible values are ASCII anyways.

> Why should we avoid changing the IID?

Because it has the potential to cause compatibility issues, and isn't strictly necessary if you avoid changing the vtable (by only adding to the end of the IDL).

> I thought about this but decided not to, at least for the time being.

I don't think your patch will compile on Linux/Windows without it...
(Reporter)

Comment 12

3 years ago
Comment on attachment 8526415 [details] [diff] [review]
Fix

To land this on branches, we should have a new interface:

interface nsIGfxInfo2 : nsIGfxInfo
{
  /**
   * Not really Gfx-related.  Here for convenience,
   * possibly only temporarily.  See bug 1102295.
   */
  readonly attribute DOMString countryCode;
};

There are several advantages of this short-term:

* only the mac implementation needs to implement the interface, so we don't need to touch other platforms
* we don't need to bump the IID of nsIGfxInfo and risk compat issues

This requires changing the existing NS_IMPL_ISUPPORTS_INHERITED declaration to add nsIGfxInfo2.

If countryCodeUTF8 is guaranteed-ASCII, then AppendASCII is fine. Otherwise this should use AppendUTF8toUTF16.

Clearing review to see the interface/IID changes.
Attachment #8526415 - Flags: review?(benjamin)
> Is there Apple documentation for that?

This is one of Gavin's questions from comment #8 that I forgot to answer.

The answer is "no" -- there isn't any Apple documentation for the user defaults in "com.apple.preferences.timezone.selected_city".  But this is yet one more case of something extremely useful (in fact for us crucial) that Apple hasn't bothered to document.  As I mentioned in comment #4, the format of this information (and the way to get it) has remained stable since at least OS X 10.6.  So it should be entirely safe to use it.  And in fact we really have no choice but to use it.
Created attachment 8527049 [details] [diff] [review]
Rev1 patch

> interface nsIGfxInfo2 : nsIGfxInfo

Benjamin, this suggestion of yours didn't work.  But this patch does work, and is (I think) a reasonable facsimile of what you want.

Note that nsIGfxInfoDebug also inherits from nsISupports (not from nsIGfxInfo).  And there are other examples in the tree (e.g. nsIContextMenuListener2) of extension interfaces that don't inherit from the interface they're extending.

I'm pretty sure none of Apple's "country codes" contain non-ASCII characters.  But on general principles I switched to using AppendUTF8toUTF16().

I did (and tested) both debug and non-debug builds on OS X -- I didn't see any problems.  I also started an all-platform set of tryserver builds, here:

https://tbpl.mozilla.org/?tree=Try&rev=b642984c1245
Attachment #8526415 - Attachment is obsolete: true
Attachment #8527049 - Flags: review?(benjamin)
(Following up comment #13)

Apple's AppKit and (private) GeoKit frameworks check the "com.apple.preferences.timezone.selected_city" user defaults, both on OS X 10.8.5 and 10.10.1 (the latest version).  And on 10.8.5 so does Apple's iMedia app, as does their Weather "extension" on 10.10.1.

So, as with their undocumented APIs, there's nothing unfinished or provisional about this stuff -- it's just that Apple reserves it for their own use.  And the fact that they *do* use it makes it much less likely to change (or disappear) suddenly.
(Reporter)

Updated

3 years ago
Attachment #8527049 - Flags: review?(benjamin) → review+
Comment on attachment 8527049 [details] [diff] [review]
Rev1 patch

Landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0d36a9b8b7d
Thanks for the quick response on this one, Steven. I decided not to take it for Firefox 34 to reduce risk/scope, but we will get it in a later release.
status-firefox34: affected → wontfix
tracking-firefox34: + → -
https://hg.mozilla.org/mozilla-central/rev/d0d36a9b8b7d
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox36: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
If this is low risk, please nominate for Beta uplift approval by Monday Dec 22 - otherwise this will be a wontfix and ship in FF36.
Flags: needinfo?(smichaud)
Comment on attachment 8527049 [details] [diff] [review]
Rev1 patch

Approval Request Comment
[Feature/regressing bug #]: na
[User impact if declined]: It won't be possible to use Chrome JavaScript code to query a Mac's country code corresponding to its (timezone-setting based) physical location
[Describe test coverage new/current, TBPL]: limited manual testing
[Risks and why]: No risks -- the new interface currently isn't used at all. But even if we do start using it, the risk that it won't operate as expected is very low.
[String/UUID change made/needed]: None
Flags: needinfo?(smichaud)
Attachment #8527049 - Flags: approval-mozilla-beta?
Attachment #8527049 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8527049 [details] [diff] [review]
Rev1 patch

Landed on mozilla-beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/caf2307e96a3

This landing was more complicated than usual in a couple of ways:

1) GfxInfoBase.cpp/h are in a different location on the 35 branch.

2) Since I've changed an *.idl file, I wasn't allowed to land on beta until I specified ba=[somebody].  Since I actually didn't change any interface, and since the original idea was Bemjamin's, I just added ba=bsmedberg.

Please let me know if I did anything wrong.
(Assignee)

Updated

3 years ago
status-firefox35: affected → fixed
> Since I've changed an *.idl file

Actually I *added* an *.idl file, but this got counted as a change.
No longer blocks: 1101642, 1102416
Blocks: 1115106
Depends on: 1310814
Blocks: 1310814
No longer depends on: 1310814
You need to log in before you can comment on or make changes to this bug.