Calendar changes the time of the events to the opposite timezone

RESOLVED FIXED in 4.4

Status

Calendar
Provider: GData
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: iampowerslave, Assigned: MakeMyDay)

Tracking

Lightning 3.3

Details

(Whiteboard: [timezone])

Attachments

(9 attachments, 3 obsolete attachments)

(Reporter)

Description

2 years ago
Created attachment 8606390 [details]
Lightning_Timezone.zip

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20150514111810

Steps to reproduce:

Created an event with Lightning
(or edited an existing event previosly created in Google Calendar or Android)

Time (24hs) : 10:00


Actual results:

The time zone in Google Calendar or Android (S Planner in my Samsung smartphone) changed.

Google calendar shows one time zone only, wrong timezone (GMT+3) with time shifted to match that timezone.

Time (24hs) : 16:00

Android shows the event with both timezones

Time (24hs) : 10:00 GMT-3
Time (24hs) : 16:00 GMT+3


Expected results:

Event should have stayed in its original timezone and time (only one timezone and the correct one) GMT - 3 and 10:00

In Calendar preferences (normal user interface) and about:config (advanced interface) show the right timezone selected in Lightning America/Argentina/Buenos Aires but it seems it calls that GMT+3 instead of GMT-3
(Assignee)

Comment 1

2 years ago
Do you have any messages in the error console (ctrl+shift+j)? Please attach the event (ics file) to this bug (you may edit it to mask private information, but please don't change any time related properties. How do you sync your Google calendar - CalDAV or with the provider for Google? Have you calendar.icaljs enabled in the advanced preferences?

And one more thing: please always attach separately to this bug (all text files at best in plain text format). Zipped files are not appreciated very much - you can attach more than one attachment if appropriate.

Can you check whether this is still an issue with the current beta to TB38/Lightning 4.0?
Whiteboard: [timezone]
(Assignee)

Updated

2 years ago
Flags: needinfo?(iampowerslave)
(Reporter)

Comment 2

2 years ago
Hi MMD. Thanks for the quick answer. Sorry about the Zip. I thought the opposite, that many images one by one would not be appreciated.

I'm in a "production environment" won't go rogue to TB38/Lightning 4.0. Sorry about that.

I'm using Provider for Google Calendar 1.0.4 to sync.

calendar.icaljs is set to false

I don't have errors, have a few warnings and messages but nothing looks related to this. Let me know how to dump it and attach it.

Same for the ICS files. I don't know where to find them. I see it is stored in SQLite.
Flags: needinfo?(iampowerslave)
(Reporter)

Comment 3

2 years ago
I have been able to export them. I was looking over the more complicated places instead of Events and Tasks->Export. I'm attaching the ICS file with two events Lightning and Splanner, one created on each name.

Only cleaned the Organizer, I believe there is nothing else to mask.
(Reporter)

Comment 4

2 years ago
Created attachment 8606436 [details]
Timezone_Issue.ics
(Assignee)

Updated

2 years ago
Attachment #8606436 - Attachment mime type: text/calendar → text/plain
(Assignee)

Comment 5

2 years ago
iampowerslave, the ics files look quite similar in trems of datetime. Can you check the error log for messages like mentioned in comment #3 of bug 1149445? eventually, you need to enable calendar.debug.log and calendar.debug.log.verbose in the config editor.

Geoff, can you take a look at this then?
Component: Lightning Only → Provider: GData
Flags: needinfo?(geoff)
Sorry, I'm unavailable for any Calendar work at the moment.
Flags: needinfo?(geoff)
(Reporter)

Comment 7

2 years ago
Created attachment 8606985 [details]
Lightning_Timezone_Changes_Log.txt

This is the log of what I have done:

1) Entered Google Calendar from the web.
Created Event "From_Google_Calendar" from 11:00 to 12:00
No time zone is set explicitly

2) Used Splanner to create a new event
"From Splanner" from 12:00 to 13:00
Time zone was explicitly set by the app as GMT-3

3) Opened Thunderbird and Console2
Events 1 and 2 synced
Created Event "From Lightning" from 13:00 to 14:00 no time zone explicitly set

4) Captured images from Google calendar that show each events timezone
will upload them separately

5) Edited events 1 and 2 and recaptured all images

6) Saved this log to txt

Regards. David.
(Reporter)

Comment 8

2 years ago
Created attachment 8606986 [details]
From_Google_Calendar.PNG
(Reporter)

Comment 9

2 years ago
Created attachment 8606987 [details]
From_Google_Calendar_Edited.PNG
(Reporter)

Comment 10

2 years ago
Created attachment 8606988 [details]
From_Lightning.PNG
(Reporter)

Comment 11

2 years ago
Created attachment 8606989 [details]
From_Splanner.PNG
(Reporter)

Comment 12

2 years ago
Created attachment 8606991 [details]
From_Splanner_Edited.PNG
(Reporter)

Comment 13

2 years ago
It looks a lot like answer 2 here http://stackoverflow.com/questions/7303580/understanding-the-etc-gmt-time-zone

ETC/GMT-3 = GMT-3 alone. Buenos Aires Argentina is in GMT-3 not GMT+3 as in

www.worldtimezone.com
(Reporter)

Comment 14

2 years ago
How do I edit a comment? I meant

ETC/GMT-3 = GMT+3
Flags: needinfo?(philipp)
(Assignee)

Comment 15

2 years ago
Created attachment 8609728 [details] [diff] [review]
FixMixedUpTimezoneOffsetGData-V1.diff

This patch takes care to detect Timezones like America/Argentina/Buenes_Aires and fixes the reverse application of the timezone offset in case of falling back to etc.
Assignee: nobody → makemyday
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8609728 - Flags: review?(philipp)
Flags: needinfo?(philipp)
(Reporter)

Comment 16

2 years ago
Thanks! Will I receive an email when this goes to a release version?
Comment on attachment 8609728 [details] [diff] [review]
FixMixedUpTimezoneOffsetGData-V1.diff

Review of attachment 8609728 [details] [diff] [review]:
-----------------------------------------------------------------

I hope we don't have other calendar code that expects just two components, I wasn't aware there are olson timezones like America/Argentina/Buenos_Aires. Nothing for this bug though.

Do you think you could add a unit test for this?
Attachment #8609728 - Flags: review?(philipp) → review+
(In reply to iampowerslave from comment #16)
> Thanks! Will I receive an email when this goes to a release version?

You will receive an email for changes to this bug, but not specifically when its released on addons.mozilla.org. You will receive an automatic update in-app though, unless you disabled it.
(Assignee)

Comment 19

2 years ago
Do you want a separate test here or something in test_gdata?
Something in test_gdata should be fine.
(Assignee)

Comment 21

2 years ago
Created attachment 8616390 [details] [diff] [review]
FixMixedUpTimezoneOffsetGData-V2.diff

This is the patch with an added test, although I'm not sure whether this test is applied correctly. I've started a try build [1] with a previous version of the test, which was wrong for the date only test, but it passed while it shouldn't. Any thought's on that?

Additionally, I found an inconsistency in the alias definition for Argentina Standard Time in zones.json: it resolved to America/Buenos_Aires, but there's only (the correct) zone definition for America/Argentina/Buenos_Aires. Should we handle this here or move it to a separate bug? I guess Geoff added the aliases for the common tz names by hand. I haven't checked for further aliases, but maybe it's worth to add a check whether all aliases resolve to a zone definition (in a separate bug).

[1] https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d440ca9639a7
Attachment #8616390 - Flags: feedback?(philipp)
(Assignee)

Comment 22

2 years ago
Created attachment 8629700 [details] [diff] [review]
FixMixedUpTimezoneOffsetGData-V3.diff

I have separated the timezone definition correction, because this not just affects gdata provider.

The test passes, so requesting review now for this.
Attachment #8609728 - Attachment is obsolete: true
Attachment #8616390 - Attachment is obsolete: true
Attachment #8616390 - Flags: feedback?(philipp)
Attachment #8629700 - Flags: review?(philipp)
(Assignee)

Updated

2 years ago
Depends on: 1180522
Comment on attachment 8629700 [details] [diff] [review]
FixMixedUpTimezoneOffsetGData-V3.diff

Review of attachment 8629700 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the elaborate patch! I'll make sure to include this in 1.0.5.

::: calendar/test/unit/test_gdata_provider.js
@@ +469,5 @@
> +add_test(function test_dateToJSON() {
> +    let tzProvider = cal.getTimezoneService();
> +    let localTz = Services.prefs.getCharPref("calendar.timezone.local") || null;
> +    
> +    function _createDateTime(timezone) {

Whitespace snuck in here.

@@ +496,5 @@
> +    equal(dateToJSON(dt), {"dateTime": "2015-01-30T12:00:00", "timeZone": "Unknown/Olson/Timezone"});
> +
> +    // invalid non-Olson tz 
> +    dt = _createDateTime(tzProvider.getTimezone("InvalidTimeZone"));
> +    notEqual(dateToJSON(dt), {"dateTime": "2015-01-30T12:00:00", "timeZone": "InvalidTimeZone"});

I wonder if we should be sending this, its bound to fail. Maybe just assume floating or don't send a timeZone and use the UTC offset in the dateTime? I believe that is valid.

Also, another whitespace snuck in here.
Attachment #8629700 - Flags: review?(philipp) → review+
(In reply to MakeMyDay from comment #21)
> Created attachment 8616390 [details] [diff] [review]
> FixMixedUpTimezoneOffsetGData-V2.diff
> 
> This is the patch with an added test, although I'm not sure whether this
> test is applied correctly. I've started a try build [1] with a previous
> version of the test, which was wrong for the date only test, but it passed
> while it shouldn't. Any thought's on that?
The test looks totally fine, not sure why it would be passing with an error. Not sure I understood your later comments correctly, did you manage to fix this?


> 
> Additionally, I found an inconsistency in the alias definition for Argentina
> Standard Time in zones.json: it resolved to America/Buenos_Aires, but
> there's only (the correct) zone definition for
> America/Argentina/Buenos_Aires. Should we handle this here or move it to a
> separate bug? 
I'm fine with handling it here.

> I guess Geoff added the aliases for the common tz names by
> hand. I haven't checked for further aliases, but maybe it's worth to add a
> check whether all aliases resolve to a zone definition (in a separate bug).
We could do this in the timezone update python script, this way its caught early. Separate bug is fine of course.
(Assignee)

Comment 25

2 years ago
Created attachment 8630702 [details] [diff] [review]
FixMixedUpTimezoneOffsetGData-V4.diff

Whitespaces removed.

The test is intended as a negative test, I kept it.
Attachment #8629700 - Attachment is obsolete: true
Attachment #8630702 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Pushed to comm-central changeset 264334727af9
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 4.4
You need to log in before you can comment on or make changes to this bug.