When changing from a daylight saving timezone to a UTC offset format timezone (in my case, "America/New York" to "UTC+08:00"), localtime_r will still return the datetime for the old timezone. A minimal fix is attached, but to avoid diversity with upstream, the preferred fix is update the file to commit 906eb99 [1]. [1] https://android-review.googlesource.com/#/c/98649/

BTW this only happens if the process keeps alive when timezone changes. New processes calling localtime_r() after timezone changes will get correct result, so the toolbox "date" command is not impacted.

It seems Android rarely use "std offset" format timezone, instead the GSM Service implementation [1] uses MCC in conjunction with NITZ offset to get a best match city defined in [2]. For instance, Firefox got "UTC+08:00" while Android uses best match city "Asia/Taipei" directly. That might explain why Android doesn't have the same symptom as Firefox OS. [1] http://androidxref.com/5.1.1_r6/xref/frameworks/opt/telephony/src/java/com/android/internal/telephony/gsm/GsmServiceStateTracker.java#1735 [2] http://androidxref.com/5.1.1_r6/xref/frameworks/base/core/res/res/xml/time_zones_by_country.xml

As another reference, the upstream of bionic timezone code and database is from IANA [1][2] [1] http://www.iana.org/time-zones/repository/tzcode-latest.tar.gz [2] http://www.iana.org/time-zones/repository/tzdata-latest.tar.gz

Changing the title as I'm noticing it's not only happen on localtime() call.

Although not many, it seems mozilla did add patches to bionic[1] before. Just checked repo manifests, emulator and many other ICS devices are using mozilla's repository instead of the untouched CAF repo. Maybe we should create another branch for KK and update manifest to point KK builds to mozilla's repository. [1] https://github.com/mozilla-b2g/platform_bionic/pulls?q=is%3Apr+is%3Aclosed

Update the tech details: For timezones using Olson format (such as "Asia/Taipei") defined in IANA timezone database [1], tzload [2] can parse the information from tzdata and everything works. However when using POSIX "std offset" format (such as "UTC+8" or "EST-5"), it goes through tzparse [3] to construct timezone info dynamically as it doesn't match tzdata records. 2 bugs related to tzparse feature are found: 1. defaulttype was not reset in tzparse(), hence when moving from a DST (daylight saving time) timezone to "std offset" timezone, it incorrectly keeps using the DST field, and causes out-of-date offset being used when calling localtime_r(). This should only affect non-Olson format timezones. 2. timezone variable is reset to 0 if timecnt is 0 when invoking tzset(). "timecnt" indicates the number of time transitions -- i.e. all DST clock changes and historical timezone changes recorded in the database. Obviously for a "std offset" timezone, timecnt is always 0 since it doens't include any DST information. This also affects countries never changed timezone or applied DST. Try "Indian/Cocos" for instance, it should be UTC+06:30 while the status bar shows UTC time. [1] https://www.iana.org/time-zones [2] https://github.com/android/platform_bionic/blob/android-4.4.4_r1.0.1/libc/tzcode/localtime.c#L343-L345 [3] https://github.com/android/platform_bionic/blob/android-4.4.4_r1.0.1/libc/tzcode/localtime.c#L930-L932

Hi Michael, We would like to phase-in the fix to B2G/KitKat. Could you possibly help to create a branch of kitkat on platform_bionic [1] repository so we can create a pull request for it? The current kitkat branch of code aurora repo [2] is the same as tag "android-4.4.2_r1" of AOSP repo [3]. [1] https://github.com/mozilla-b2g/platform_bionic [2] git://codeaurora.org/platform/bionic [3] https://github.com/android/platform_bionic/tree/android-sdk-4.4.2_r1

I think it's fine to patch our own copy of bionic. However, it's our own copy, and the SoC vendors needs to be notified to take the patches in their forks. What version of Android is this fixed in? Can we create some sort of test for this so devices without this patch will fail?

NI Carol to put this on her radar.

Thanks for the heads up Hsinyi! Adding Phil here to track this.

This seems very similar to something we noticed last last year when we were working on KK - see bug 1000352 comment 9 and bug 1000352 comment 12. We resolved this issue by applying a similarly tiny patch to bionic in our releases. The good news is that there's been a lot of changes in localtime.c in the lollipop baseline and it seems switching between timezone formats is no longer an issue.

:freesamael, would you be interested in assigning yourself to this bug? It seems like you're already working on it. Thanks!

Why do we persist relying on broken features ? Do we intend to fork this code for all and every device we want to support ?

triage: this is blocking a bunch of blockers.

(In reply to Alexandre LISSY :gerard-majax from comment #17) > Why do we persist relying on broken features ? Do we intend to fork this > code for all and every device we want to support ? We've got the same concern, but unfortunately there's no easy workaround. Android didn't suffer that too much with the price of keeping 3 different timezone databases on application [1] / framework [2] / system [3] level respectively. Firefox OS inherently keeps tzdata [3], and Gaia has an extra tz.json [4]. With only 2 separate databases, we've already having synchronization issues -- try to select "Asia/Ürümqi" from your device, the device time will change to UTC time instead of UTC+8. The reason is tzdata uses "Urumqi", not "Ürümqi", so the name matches no record and fallbacks to UTC. Even Android has sort of workaround, it still suffers sometimes. Google complained that too: > NITZ is fundamentally a broken idea. [5] Another problem of Android is that to avoid database synchronization issue, Android keeps only minimal set of timezones for user to choose [1], and that sometimes feels confusing. Users are complaining the cities of their countries are not available [6]. Before we have a well-designed solution across Gaia, Gecko and Gonk, fixing this bionic bug looks a promising choice. In the meantime, I'm looking around Android timezone related bugs. If we can identify one or more Android issues caused by the same bug, we can contribute the fix back to upstream, which will benefit both projects. [1] http://androidxref.com/5.1.1_r6/xref/packages/apps/Settings/res/xml/timezones.xml [2] http://androidxref.com/5.1.1_r6/xref/frameworks/base/core/res/res/xml/time_zones_by_country.xml [3] http://androidxref.com/5.1.1_r6/xref/bionic/libc/zoneinfo/tzdata (binary format) [4] https://github.com/mozilla-b2g/gaia/blob/01ffe82cf088ca8fda9fe6783dc5cad2c3dde01c/shared/resources/tz.json [5] https://code.google.com/p/android/issues/detail?id=62419 [6] https://code.google.com/p/android/issues/detail?id=182823

(In reply to Michael Wu [:mwu] from comment #9) > I think it's fine to patch our own copy of bionic. However, it's our own > copy, and the SoC vendors needs to be notified to take the patches in their > forks. > > What version of Android is this fixed in? Can we create some sort of test > for this so devices without this patch will fail? Hi Michael, I've created an automation test case for emulator, as attached, along with necessary patches in Bug 1212238 and Bug 1212240 to make the test run. Unfortunately I can't make the test run on real devices, as the symptom involves NITZ update from operator networks which can not be mocked on real devices. But at least we can ensure not forgetting cherry-pick the patch in the future when we move to a newer version of AOSP. The fix is for kitkat, as mentioned in Comment 8. Could you help to create a B2G kitkat branch which I can create a pull request upon?

I wonder if this would help with bug 1207139

and bug 1210093

Michael, any update?

Sorry for the delay - the bionic branch has been created.

Comment on attachment 8677833 [details] Fix settzname & tzparse for "std offset" timezones. r=tzimmermann Hi Michael, I'm not quite sure who I sure ask for reviewing this patch. Would you possibly help to review this patch or point me someone proper to review this patch?

Comment on attachment 8678055 [details] [diff] [review] (v4) Add marionette test case Hi Edgar, Could you help to review the test case along with the emulator patch?

Comment on attachment 8677833 [details] Fix settzname & tzparse for "std offset" timezones. r=tzimmermann Hi Thomas, Could you help to review this bionic patch? Thanks a lot.

Comment on attachment 8678055 [details] [diff] [review] (v4) Add marionette test case Review of attachment 8678055 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed or answered. Thank you. ::: dom/system/gonk/tests/marionette/test_nitz_timezone_changes.js @@ +57,5 @@ > +} > + > +function testChangeTimezone(aTzdiff, aDateString, aTimeString) { > + return runEmulatorCmdSafe('gsm timezone ' + aTzdiff) > + .then(() => waitForTimezoneUpdate(aTzdiff, aDateString, aTimeString)); We usually register listener first to prevent possible racing. And in order to do so, we probably need to use Promise.all(). @@ +63,5 @@ > + > +// Start test > +startTestBase(function() { > + return init() > + .then(() => testChangeTimezone(36, '01/02', '09:00')) // UTC+09:00 Nit: The expected Date and Time would be "01/02" and "09:00" is because we use the 'UTC 00:00:00, 2nd of Jan, 1970' as a base line in waitForTimezoneUpdate(). It probably make the code clearer if we could put them in waitForTimezoneUpdate(). Besides, do we really need to hard code them, couldn't they just be calculated by timezone diff?

Comment on attachment 8677833 [details] Fix settzname & tzparse for "std offset" timezones. r=tzimmermann I left a comment on Github, but I have no knowledge of the timezone code, so I cannot give you a good review here.

Comment on attachment 8678767 [details] [diff] [review] (v5) Add marionette test case Hi Edgar, I've updated the test script for better test coverage by adding some Olson timezones. Would you like to take a second look? I was also willing to add some countries who have changed their timezone historically (not including DST), but apparently the timezones I tried (Asia/Kathmandu and Asia/Colombo) do have different records in tzdata used by bionic and zoneinfo64.txt used by ICU...

Comment on attachment 8678767 [details] [diff] [review] (v5) Add marionette test case Review of attachment 8678767 [details] [diff] [review]: ----------------------------------------------------------------- It is always good to have more tests. Thank you, Samael. ::: dom/system/gonk/tests/marionette/test_timezone_changes.js @@ +66,5 @@ > + return new Promise(function(aResolve, aReject) { > + window.addEventListener('moztimechange', function onevent(aEvent) { > + // Use 'UTC 00:00:00, 2nd of Jan, 1970' by default. > + let testDate = (aTestDateInMillis !== undefined) ? > + new Date(aTestDateInMillis) : new Date(86400000); Nit: Or you can give |aTestDateInMillis| a default value, e.g. function waitForTimezoneUpdate(aTzOffset, aTestDateInMillis = 86400000, ...) { ... } @@ +76,5 @@ > + window.removeEventListener('moztimechange', onevent); > + > + // The UTC time of offsetDate is the same as the expected local time of > + // testDate. We'll use it to verify the values. > + let offsetDate = new Date(testDate.getTime() - aTzOffset * 60 * 1000); Nit: maybe (aTestDateInMillis - aTzOffset * 60 * 1000) @@ +85,5 @@ > + testDate = new Date(aTransTestDateInMillis); > + is(testDate.getTimezoneOffset(), aTransTzOffset); > + > + // Verify DST date. > + offsetDate = new Date(testDate.getTime() - aTransTzOffset * 60 * 1000); Ditto: (aTransTestDateInMillis - aTransTzOffset * 60 * 1000)

(In reply to Samael Wang [:freesamael][:sawang] from comment #44) > I was also willing to add some countries who have changed their timezone > historically (not including DST), but apparently the timezones I tried > (Asia/Kathmandu and Asia/Colombo) do have different records in tzdata used > by bionic and zoneinfo64.txt used by ICU... Working on multiple timezone issues simultaneously gets myself fooled. The reason that these 2 cities didn't work is because of bug 1210093. At least we know what to test there.

b2g-4.4.2_r1: https://github.com/mozilla-b2g/platform_bionic/commit/3e85c4683c121530c1c3a48c696a569bf5f587e2

Hi Edgar, Could you help to review the manifests update?

master: https://github.com/mozilla-b2g/b2g-manifest/commit/fbaeab07d17ce4049fe8f871a3f55942af29bc4e

Good news here. I wrote a tiny test program [1] against latest tzcode / tzdata from IANA, and also reviewed IANA's localtime.c [2] implementation. I can confirm the 2 bugs mentioned in comment 7 have both been fixed by IANA. It doesn't seem to be merged by Android yet as of Marshmallow, but we have great opportunity seeing Android to adapt the fix in future releases. [1] https://github.com/freesamael/timezone/blob/d0012c8ce1988d9d2c4ebca5ca46eff9f4e917ea/iana-tz2015g/tzbug.c [2] https://github.com/freesamael/timezone/blob/d0012c8ce1988d9d2c4ebca5ca46eff9f4e917ea/iana-tz2015g/localtime.c

I'm not quite sure what I'm doing is considered a reproduction of this issue, but I can get a running app on Flame 2.5 to show a time that is not offset correctly for the timezone. 1) Open the clock app 2) Change the view to the digital view 3) Open Settings and change the timezone by changing the region. 4) Return to the clock app. Results: The user is shown the previously set timezones time and not the current one. The clock face view will correctly show the time change (you can freely change between them) and this issue can also be seen when setting an alarm (the overall page will show the previous timezone, but if you select it to change the time, it will show the current timezone) as well. Killing the clock app with the task manager and reopening it will fix this issue. Environmental Variables: Device: Flame 2.5 BuildID: 20151104004502 Gaia: 91cac94948094cfdcd00cba5c6483e27e80cb3b0 Gecko: f14715d84261aa5da68680bc889734f923d5d4c7 Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a Version: 44.0a1 (2.5) Firmware Version: v18D User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

There may be two reasons for this behavior. One is that Intl API doesn't update timezone internally, and another that Gaia level IntlHelper doesn't reset after timezone change. I just landed the fix for the latter (bug 1221817). Please, retest with current master if possible to confirm that it's not the IntlHelper fault.

(In reply to Jayme Mercado [:JMercado] from comment #58) > I'm not quite sure what I'm doing is considered a reproduction of this > issue, but I can get a running app on Flame 2.5 to show a time that is not > offset correctly for the timezone. > > 1) Open the clock app > 2) Change the view to the digital view > 3) Open Settings and change the timezone by changing the region. > 4) Return to the clock app. > > Results: The user is shown the previously set timezones time and not the > current one. The clock face view will correctly show the time change (you > can freely change between them) and this issue can also be seen when setting > an alarm (the overall page will show the previous timezone, but if you > select it to change the time, it will show the current timezone) as well. > Killing the clock app with the task manager and reopening it will fix this > issue. My local test shows it's a regression issue. See bug 1208808 comment 44.

Zibi: I'm seeing the same results on today's master Flame and Aries builds that I was seeing yesterday. Environmental Variables: Device: Aries 2.6 BuildID: 20151106140230 Gaia: cec4c1d3729137a24163756d15f98b0d37803966 Gecko: 512caeeb5565bb819a24ebda6a37b44fdf71b6c3 Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56 Version: 45.0a1 (2.6) Firmware Version: D5803_23.1.A.1.28_NCB.ftf User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0 Environmental Variables: Device: Flame 2.6 BuildID: 20151106030214 Gaia: f39a7a827c0c0f48087ff3ead94f61ae22523919 Gecko: cc48981c026c50fdf80d47b040ae1fb8fe99ad07 Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a Version: 45.0a1 (2.6) Firmware Version: v18D User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Can we make sure this is fixed on AOSP Lollipop too? I do see the issue on my builds for several Sony devices :(

See comment 62 above

Jayme - I need STR for that bug you are seeing. It may be worth filing a new bug to not spam this one with a potential separate issue.

Please see comment 58 for steps. I'm still seeing this issue on today's build. It may be bug 1208808 instead but if that's the case I don't know how to test this issue separate.

Ok. So changing *timezone* will not work because of bug 1208808. Changing *time* should. So no new bug here, we're waiting for the regression in bug 1208808 to be fixed.

(In reply to Alexandre LISSY :gerard-majax from comment #62) > Can we make sure this is fixed on AOSP Lollipop too? I do see the issue on > my builds for several Sony devices :( We can technically phase-in the same solution, but we're not able to make automated testing on Android L as we don't have stable emulator-l for automation. I'm not sure if we should do this without being able to run a test case for it. You can try to cherry-pick the commit for local build but you'll encounter the regression mentioned in bug 1208808 comment 44 afterward.

(In reply to Alexandre LISSY :gerard-majax from comment #62) > Can we make sure this is fixed on AOSP Lollipop too? I do see the issue on > my builds for several Sony devices :( Sure. :)

(In reply to Samael Wang [:freesamael][:sawang] from comment #67) > (In reply to Alexandre LISSY :gerard-majax from comment #62) > > Can we make sure this is fixed on AOSP Lollipop too? I do see the issue on > > my builds for several Sony devices :( > > We can technically phase-in the same solution, but we're not able to make > automated testing on Android L as we don't have stable emulator-l for > automation. I'm not sure if we should do this without being able to run a > test case for it. > > You can try to cherry-pick the commit for local build but you'll encounter > the regression mentioned in bug 1208808 comment 44 afterward. What I see is a wrong time after getting NITZ on the first boot. Rebooting fixes that. So on my device, I see: - xxx 12:38:00 before getting NITZ - today, 15:15:00 after getting NITZ - today, 16:15:00 after rebooting