Leo Reporter
	Description • 11 years ago

Steps to reproduce

1.Performed a factory reset; 
2.Advance to get date & time;
3.Check the continent and city/country.


Expected behavior: The handset should display the continent default: "America" and city: Brasilia.

Actual:
The handset displays Oceano Pacifico and Pago Pago as default.

	Andreas Gal :gal
	Comment 1 • 11 years ago

Uploaded untested patch.

https://github.com/mozilla-b2g/gaia/pull/9883

	Andreas Gal :gal
	Comment 2 • 11 years ago

Assigning to Kaze for now since I won't have time for this for the next day or so. He will unassign or re-assign as needed.

	Rudy Lu [:rudyl] (inactive after 2015/7/3)
	Comment 3 • 11 years ago

Hi,

I am willing to take a look at this if Kaze let me.

Hi Kaze,

Have you started working on this?
Thanks.

	Andreas Gal :gal
	Comment 4 • 11 years ago

I just assigned to Kaze so its assigned to someone. Go ahead Rudy.

	Rudy Lu [:rudyl] (inactive after 2015/7/3)
	Comment 5 • 11 years ago

Kaze mentioned on IRC that we may want to propose an alternative way to our current UX design of timezone selection.

I think it will be much like that on Android (See the attachment).
--
If we are trying to take that design, it would be a lot easier for us to do 
"time zone offset" -> "time zone UI item" transformation.
(We will only get time zone offset from NITZ)

	Rudy Lu [:rudyl] (inactive after 2015/7/3)
	Comment 6 • 11 years ago

I think we needs this mcc/mnc-based guessing only when NITZ is not available, right?
If that is the case, I think we should create a separate bug for the "simpler time zone list" in Comment 5 and do the guessing here.

Hi Andreas,

Do you think my understanding is correct?
Thanks.

	Fabien Cazenave [:kaze] Assignee
	Comment 7 • 11 years ago

One thing I don’t understand in this patch is that the `mccmnc.json' file proposes a country match for every MCC/MNC combination. As far as I know the MNC could be safely ignored to make this match, is that correct?

In which case I’d like to propose a simpler approach, by keeping the existing `tz.json' file unchanged and creating an `mcc.json' file that proposes a default timezone for each MCC value, e.g.:

{
  "208": {
    "cc": "FR",
    "tz": "Europe/Paris"
  },
  "310": {
    "cc": "US",
    "tz": "America/New_York"
  },
  […]
}

Then the tz_selector() could rely on this `mcc.json' database to propose a default timezone when necessary. Of course this won’t always work for countries that have several timezones — in fact it won’t always be enough to select the region (= continent or ocean): "310" (US) could match America/New_York or Pacific/Honolulu.

(In reply to Rudy Lu [:rudyl] from comment #6)
> I think we needs this mcc/mnc-based guessing only when NITZ is not
> available, right?

If I understand correctly, this is right only if we choose to rely on GMT offsets rather than on “Region/City” identifiers: LTIC the NITZ info only gave a GMT offset.

For the record, the first UX wireframes we had suggested to choose the timezone on a GMT-offset basis rather than on a region/city one, but it didn’t work: the back-end recognized region/city identifiers but ignored the GMT offset value. That’s why we ended up with this kind of identifiers.

However, after some thoughts (including bug 857545) I’m not sure we should switch to GMT offsets right now. So I’d suggest to be smarter guessing a proper region/city identifier right now, and open another bug to think of switching to a GMT-offset identifier instead.

	Fabien Cazenave [:kaze] Assignee
	Comment 8 • 11 years ago

(In reply to Fabien Cazenave [:kaze] from comment #7)
> One thing I don’t understand in this patch is that the `mccmnc.json' file
> proposes a country match for every MCC/MNC combination. As far as I know the
> MNC could be safely ignored to make this match, is that correct?

I found two exceptions to this rule:
 • MCC=234 can match "GB, "JE" (Jersey) or "IM" (Isle of Man)
 • MCC=515 can match "PH" (Indian/Manila, GMT+8) or "PK" (Asia/Karachi, GMT+5)

The first case is OK (both JE and IM timezones are an alias to Europe/London) but the latter would be a blocker.

	Fabien Cazenave [:kaze] Assignee
	Comment 9 • 11 years ago

Ironing a few things, sorry for the delay.

	Fabien Cazenave [:kaze] Assignee
	Comment 10 • 11 years ago

https://github.com/mozilla-b2g/gaia/pull/9965

This PR is based on Andreas’ work. I did the required bugfixes and I tried to minimize the changes: the shared `apn.json' and `tz.json' files are unchanged, and a new `apn_tz.json' database is created to map every mcc/mnc tuple directly to a timezone.

  {
    "202": "Europe/Athens", 
    "204": "Europe/Amsterdam", 
    […]
    "510": "Asia/Jakarta", 
    "515": {
      "01": "Asia/Karachi", 
      "02": "Asia/Manila", 
      "03": "Asia/Manila", 
      "04": "Asia/Karachi", 
      "05": "Asia/Manila", 
      "06": "Asia/Karachi", 
      "07": "Asia/Karachi"
    }, 
    "520": "Asia/Bangkok", 
    […]
    "744": "America/Asuncion", 
    "748": "America/Montevideo"
  }

This saves us a few kB in shared/resources and a few JS lines in shared/js.

	Andreas Gal :gal
	Comment 11 • 11 years ago

Looks good to me. Thanks.

	Rudy Lu [:rudyl] (inactive after 2015/7/3)
	Comment 12 • 11 years ago

Comment on attachment 753585 [details]
link to pull request

Hi Kaze,

Thanks for working on this.

1. Maybe we should favor the mcc/mnc guessing over the setting entry "time.timezone.user-selected" (from Bug 847342) whenever when we got mcc/mnc info available.
   For current logic, we would always use "timezone.user-selected" value first (if it is not null), right?

2. Please note that when you invoke tzSelect() to init a selector for time zone, the init process will not invoke setTimezone().
   Take the "NITZ is not available" case, and mcc/mnc is available, the UI of FTE may show the correct value in TZ selector (say Asia/Taipei) based on mcc/mnc, however, since setTimezone() was not invoked, the actual time zone would not be set. -> UI and backend info not synced.

3. When the user set "time auto update" as disabled, we would restore the time zone to "time.timezone.user-selected", maybe we should modify this part to mcc/mnc guessing as well.
https://github.com/mozilla-b2g/gaia/blob/2a25741af44be31e742851eda7df9e643cdcff2c/apps/settings/js/date_time.js#L107

--
Please let me know if you think anything above is unclear or need me to do the follow-up patch for any of them.
Thanks. 

(review flag cleared so that I will get notified if you reset it again)

	Andreas Gal :gal
	Comment 13 • 11 years ago

(In reply to Rudy Lu [:rudyl] from comment #12)
> Comment on attachment 753585 [details]
> link to pull request
> 
> Hi Kaze,
> 
> Thanks for working on this.
> 
> 1. Maybe we should favor the mcc/mnc guessing over the setting entry
> "time.timezone.user-selected" (from Bug 847342) whenever when we got mcc/mnc
> info available.
>    For current logic, we would always use "timezone.user-selected" value
> first (if it is not null), right?

I don't think we should do that. The only reason that this entry is different from what the mcc/mnc suggest is that a) the user wants to run the phone in a different timezone intentionally or b) there are multiple timezones for that mcc/mnc combo. We should always go with user intent first.

> 
> 2. Please note that when you invoke tzSelect() to init a selector for time
> zone, the init process will not invoke setTimezone().
>    Take the "NITZ is not available" case, and mcc/mnc is available, the UI
> of FTE may show the correct value in TZ selector (say Asia/Taipei) based on
> mcc/mnc, however, since setTimezone() was not invoked, the actual time zone
> would not be set. -> UI and backend info not synced.

Thanks. We should change the patch to update the timezone when the dialog is displayed.

> 
> 3. When the user set "time auto update" as disabled, we would restore the
> time zone to "time.timezone.user-selected", maybe we should modify this part
> to mcc/mnc guessing as well.
> https://github.com/mozilla-b2g/gaia/blob/
> 2a25741af44be31e742851eda7df9e643cdcff2c/apps/settings/js/date_time.js#L107
> 
> --
> Please let me know if you think anything above is unclear or need me to do
> the follow-up patch for any of them.
> Thanks. 
> 
> (review flag cleared so that I will get notified if you reset it again)

	Fabien Cazenave [:kaze] Assignee
	Comment 14 • 11 years ago

Comment on attachment 753585 [details]
link to pull request

Thanks for your feedback Rudy!

(In reply to Rudy Lu [:rudyl] from comment #12)
> 1. Maybe we should favor the mcc/mnc guessing over the setting entry
> "time.timezone.user-selected" (from Bug 847342) whenever when we got mcc/mnc
> info available.

I second Andreas’ opinion here.

> 2. Please note that when you invoke tzSelect() to init a selector for time
> zone, the init process will not invoke setTimezone().

Good catch. I’ve just updated my pull request, please have another look.

> 3. When the user set "time auto update" as disabled, we would restore the
> time zone to "time.timezone.user-selected", maybe we should modify this part
> to mcc/mnc guessing as well.

This case won’t happen for the moment because the FTE screen forces the “time.timezone.user-selected” setting to a default timezone — at least, until bug 826359 is fixed — so I’d prefer to open a follow-up for this bug. A possible fix might be to move the “time.nitz.automatic-update.enabled” observer to `shared/js/tz_select.js'.

	Rudy Lu [:rudyl] (inactive after 2015/7/3)
	Comment 15 • 11 years ago

Comment on attachment 753585 [details]
link to pull request

This patch looks good to me.
Thanks.

	Fabien Cazenave [:kaze] Assignee
	Comment 16 • 11 years ago

Thanks for the review, merged on master:
https://github.com/mozilla-b2g/gaia/commit/7883dfe7a5d98bd85fc674c6e6726770c65a0157

	John Ford [:jhford] CET/CEST Berlin Time
	Comment 17 • 11 years ago

Uplifted 7883dfe7a5d98bd85fc674c6e6726770c65a0157 to:
v1-train: e0c5b9358e2c73ea44976f9c24ef486de61121d4

	John Hammink
	Comment 19 • 11 years ago

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/282b5c37cf8d
Gaia   e2ef782119b7e79fc62c48d36f0c36909d982988
BuildID 20130712070210
Version 18.0

I'm seeing America => New York; even thought that's not the correct timezone.  

Is it possible to check it in other time zones?  Reopening.

	Jason Smith [:jsmith]
	Comment 20 • 11 years ago

This patch landed almost a month and half ago. Please file a separate bug for the issue you've hit.

	John Hammink
	Comment 21 • 11 years ago

This bug is not fixed. I need to get these bugs out of my queries.   So verifying this as fixed even though it is not fixed.  - refiling. I need some way to track my work too.  

Please do not move back to resolved, since it isn't resolved.  If we can't do that, please make a better suggestion.  Thanks.