1146286 - Events from before daylight savings time change are one hour off

Status: RESOLVED FIXED

(Calendar :: Provider: GData, defect, P1)

Description

[Mark Banner (:standard8)]

I've got recurring events that were scheduled in Google (by other people inviting me), from before the US timezone change 2 weeks ago.

These events are showing as one-hour later than they should be. Newer events show at the correct time.

They do not "fix" themselves after next week's UK timezone change.

All events are showing at the right time in Google Calendar.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I see the same for my events which are scheduled by other people. I already missed a couple of meetings due to this problem.

Comment 2

[Philipp Kewisch [:Fallen][☁️📆]]

Mark, if you have additional debug info please attach. I'm looking for the JSON of events that have this problem, compared to the parsed event message which should be just a little later. Bonus points for the generated ICS data, you can get this using the ICS inspector extension.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: some debug data

I got some debug data. Maybe it's helpful.

Comment 4

[Mark Banner (:standard8)]

Remind me, which debug prefs I need to set and where the output comes.

Comment 5

[Philipp Kewisch [:Fallen][☁️📆]]

calendar.debug.log + calendar.debug.log.verbose and the messages come in the error console

Comment 6

[Ben Lerner]

Philipp, I have the same problem, but I just manually fixed all my events so they're no longer broken.  But...some interaction between Lightning(v3.3.3)/GData(v1.0.4) and Android(v4.4.4)/Calendar(v5.1) has been "resetting" the events to being broken.  If I notice it happen again, I'll try to grab the JSON.

In the meantime, I've noticed that Lightning only has "America/New York" as a time zone option, while Android Calendar offers "Eastern Daylight Time GMT-4" on my phone, and Android itself offers "GMT-05:00 Eastern Daylight Time" and Google Calendar offers "(GMT-05:00) Eastern Time".  That "GMT-4" wasn't a typo.  I wouldn't be surprised if somewhere in this mess of inconsistently named timezones is part of the problem.

Comment 7

[Ben Lerner]

I should add -- these were events I created for myself, not ones that I was invited to and accepted.  That's why I could edit them to be "fixed"...

Comment 8

[Ben Lerner]

One of my repeating events happened yesterday (with an alarm), and one did not.  I dismissed the alarm from both lightning and my phone, and that one repeating event is now displaying incorrectly; the other repeating event is still correct.

According to Google Calendar, that repeating event is now on "(GMT-08:00) America/Dawson" time (?!?)  I went to look at the individual calendar settings (in Google Calendar) and found that it was defaulting to Pacific time.  (This may be because I moved from the west coast back east, and I'd created the calendars while out west).  I've now corrected that to "GMT-05:00) Eastern time"; maybe that'll help?

Looking at the event that's still correct in Google Calendar shows that it has no time zone associated with it right now.  That's weird...

I've once again fixed the event that went wrong in Google Calendar, and re-synced Lightning.  Here's the JSON I get back (redacted slightly) for this event:
[calGoogleCalendar] Parsing entry:
{
 "kind": "calendar#event",
 "etag": "\"2848974220137000\"",
 "id": "XXXXXXXXXXXXXXXXXXXXXXXXXXXX",
 "status": "confirmed",
 "htmlLink": "https://www.google.com/calendar/event?eid=XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXxxx",
 "created": "2015-01-04T17:27:27.000Z",
 "updated": "2015-04-09T12:16:13.122Z",
 "summary": "XXXXXXXXXXXXXXXXXXXXXXXXXXX",
 "location": "XXXXXXXXXXXXXXXXXXXXXXXXXXX",
 "creator": {
  "email": "[e-mail]",
  "displayName": "Benjamin Lerner"
 },
 "organizer": {
  "email": "[e-mail]",
  "displayName": "Teaching",
  "self": true
 },
 "start": {
  "dateTime": "2015-01-12T13:35:00-05:00",
  "timeZone": "America/New_York"
 },
 "end": {
  "dateTime": "2015-01-12T14:40:00-05:00",
  "timeZone": "America/New_York"
 },
 "recurrence": [
  "EXDATE;TZID=America/New_York:20150119T133500",
  "EXDATE;TZID=America/New_York:20150216T133500",
  "EXDATE;TZID=America/New_York:20150218T133500",
  "EXDATE;TZID=America/New_York:20150309T133500",
  "EXDATE;TZID=America/New_York:20150311T133500",
  "EXDATE;TZID=America/New_York:20150312T133500",
  "EXDATE;TZID=America/New_York:20150420T133500",
  "RRULE:FREQ=WEEKLY;UNTIL=20150422T173500Z;BYDAY=MO,WE,TH"
 ],
 "iCalUID": "[e-mail]",
 "sequence": 37,
 "extendedProperties": {
  "private": {
   "X-MOZ-LASTACK": "2015-04-08T19:06:09Z"
  }
 },
 "reminders": {
  "useDefault": false,
  "overrides": [
   {
    "method": "popup",
    "minutes": 10
   }
  ]
 }
}
[calGoogleCalendar] Logging calIEvent:
	id:[e-mail]
	created:2015/01/04 17:27:27 UTC isDate=0 nativeTime=1420392447000000
	updated:2015/04/09 12:16:13 UTC isDate=0 nativeTime=1428581773000000
	title:XXXXXXXXXXXXXXXXXXXXXXXX
	description:null
	transparency:null
	status:CONFIRMED
	startTime:2015/01/12 13:35:00 America/New_York isDate=0 nativeTime=1421087700000000
	endTime:2015/01/12 14:40:00 America/New_York isDate=0 nativeTime=1421091600000000
	location:XXXXXXXXXXXXXXXXXXXXXXXXXX
	privacy:null
	sequence:37
	alarmLastAck:2015/04/08 19:06:09 UTC isDate=0 nativeTime=1428519969000000
	snoozeTime:null
	isOccurrence: false
	Organizer: 
		ID: mailto:[e-mail]
			Name: Teaching
			Rsvp: null
			Is Organizer: yes
			Role: null
			Status: null
	Attendees: 
	recurrence: yes: 
		EXDATE:20150119T183500Z
		EXDATE:20150216T183500Z
		EXDATE:20150218T183500Z
		EXDATE:20150309T173500Z
		EXDATE:20150311T173500Z
		EXDATE:20150312T173500Z
		EXDATE:20150420T173500Z
		RRULE:FREQ=WEEKLY;UNTIL=20150422T173500Z;BYDAY=MO,WE,TH
	Exceptions:

	alarms: yes: 
		
		Action: DISPLAY
		Offset: -PT10M
		alarmDate: null
		related: 1
		repeat: 0
		repeatOffset: null
		repeatDate: null
		description: null
		summary: null
		properties: no


(I also get some strangeness involving exceptions to the recurring nature of the event: besides the EXDATEs listed above, I also get individual events with status="cancelled" for each of the EXDATEs. Seems redundant to me.)

When today's events happen, I'll post again with the updated JSONs.

Comment 9

[Ben Lerner]

So something went wrong with my phone today, and didn't give me any alerts at all.  But Lightning did, and I dismissed them as I usually do.  When I logged into Google Calendar, the events no longer have a timezone assigned, even though they are being requested in the JSON above as America/New_York.

No idea what's going on...

Comment 10

[Philipp Kewisch [:Fallen][☁️📆]]

Also reported at https://groups.google.com/forum/#!topic/provider-for-google-calendar/6PxbCWNHqBQ

I believe the problem may be that sometimes UTC is assumed to be Europe/London. In the mentioned link, the following shows up:

  "dateTime": "2015-03-16T13:00:00Z",
  "timeZone": "Europe/London"


The "Z" is for UTC, while a timezone is also specified.

Comment 11

[barney]

Can I add that the same thing happens when Creating an event through the provider.  If you create an event on a day that is in BST then the TZ shows as Europe/London. But when you create an event on a day that is in GMT then even though the event is created with a TZ of Europe/London when you look at the event again or in the Google Calendar it shows as UTC/GMT.


"start": {
  "dateTime": "2016-01-07T19:15:00Z",
  "timeZone": "Europe/London"
 },
 "end": {
  "dateTime": "2016-01-07T20:15:00Z",
  "timeZone": "Europe/London"
 },


created:2015/08/01 08:37:01 UTC isDate=0 nativeTime=1438418221000000
	updated:2015/08/01 08:37:01 UTC isDate=0 nativeTime=1438418221000000
	title:testing
	description:null
	transparency:null
	status:CONFIRMED
	startTime:2016/01/07 19:15:00 UTC isDate=0 nativeTime=1452194100000000
	endTime:2016/01/07 20:15:00 UTC isDate=0 nativeTime=1452197700000000
	location:null
	privacy:null
	sequence:0
	alarmLastAck:null
	snoozeTime:null
	isOccurrence: false

Xml from the event :

<title type='html'>testing</title><summary type='html'>When: Thu Jan 7, 2016 7:15pm to 8:15pm&amp;nbsp;
GMT&lt;br&gt;


&lt;br&gt;Event Status: confirmed</summary><content type='html'>When: Thu Jan 7, 2016 7:15pm to 8:15pm 
GMT&lt;br /&gt;


&lt;br /&gt;Event Status: confirmed</content>

Comment 12

[Alan.Burlison]

I'm seeing the same issue with reoccurring events in my Google calendar that I set up long before the DST change - they are all 1 hour out. I'm in the UK timezone and the events are in US Pacific. The events all display correctly in the Google Calendar web interface, including across the DST boundaries but are off by one hour in Lightning. This makes Lightning useless for me as I work cross-timezone.

Comment 13

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Fix - v1

I do not yet know if this patch will fix the issue, need some sleep before I test it. If not I will attach this patch to another bug. Maybe someone is around that is comfortable with applying patches :)

Comment 14

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Fix - v2

Ok, after some testing I figured out the real issue. Google is sending dates with a "Z" at the end, but also a timezone. Lightning gives the "Z" priority and it ends up being UTC. This patch fixes the issues from the previous patch, plus the one mentioned.

Comment 15

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: gdata-provider-1.0.5pre2.xpi

Here is an xpi with the patch applied, plus a few others (bug 1189109, bug 1169062).

btw, I've chosen to NOT add any migration code. I can't differ between an event deliberately put into UTC and one put into UTC by this bug, and the only other option I have is resetting all calendars if the user is in one of those timezones. I'd rather not reset the calendars if I don't have to because it will cause quite a lot of extra API requests.

If you have old meetings where this bug is happening, please re-subscribe to the calendar.

Comment 16

[Mark Banner (:standard8)]

This doesn't seem to be working for me properly. I've reinstalled Google Calendar, but some of my events are now right, but some of the older ones are now e.g. at midnight rather than at 4pm - i.e. I don't think the timezone is being accounted for at all.

Comment 17

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 8644866 [details] [diff] [review]
Fix - v2

Indeed, this doesn't work as expected.

Comment 18

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Fix - v3

Next try, this should do it. I'll attach a new test xpi in a sec.

Comment 19

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: gdata-provider-1.0.5pre3.xpi

Comment 20

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 8645371 [details] [diff] [review]
Fix - v3

...and of course I find another issue just after I upload the patch

Comment 21

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Fix - v4

Ok, next try. The date field is a little tricky to understand. From what I've understood now, the "dateTime" member is ALWAYS in the timezone specified by the timeZone request query parameter, even if a "timeZone" field is specified in the JSON date object. If the query parameter is not specified (or on the responses to the add/modify requests), then the timezone that is set for the calendar (in Google Settings) is used.

When added via Lightning, the "timeZone" is specified in the date object, but this is not the case for events created via Google Calendar in the web. In that case I assume the event is meant to have the timezone of the calendar.

I've therefore decided to not request the date in a specific timezone, but instead always use Google's timezone setting for the calendar. If a "timeZone" is then specified in the JSON date object, then I use getInTimezone to convert to that zone.

This has been tested my Google Calendar set to America/Los_Angeles, Europe/London, Europe/Berlin and Asia/Baku, creating events both through Lightning and via web.

I wouldn't be surprised if there is a case I missed, so I'd appreciate some testing in the xpi I'll attach next. Please remember to reset you calendar after installing.

Comment 22

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: gdata-provider-1.0.5pre4.xpi

Comment 24

[[:MakeMyDay]]

Comment on attachment 8645744 [details] [diff] [review]
Fix - v4

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

Finally, the review. Sorry that it took so long.

Did this pass? I'm wondering whether that test was really applied before. Can you verify this locally by making one test step to fail and see whether the whole test then fails?

r+ given the below comments are considered and the the test works.

::: calendar/providers/gdata/modules/gdataUtils.jsm
@@ +235,5 @@
> +        dateTime.second = matches[7];
> +    }
> +
> +    dateTime.timezone = aTimezone;
> +    if (matches[9] != null) {

if (match[9]) {

should be sufficient here.

@@ +237,5 @@
> +
> +    dateTime.timezone = aTimezone;
> +    if (matches[9] != null) {
> +        let offset_in_s = 0;
> +        if (matches[11] != null) {

Again: if (match[11]) {

@@ +256,5 @@
> +}
> +fromRFC3339FixedZone.regex = new RegExp(
> +    "^([0-9]{4})-([0-9]{2})-([0-9]{2})" +
> +    "([Tt]([0-9]{2}):([0-9]{2}):([0-9]{2})(\\.[0-9]+)?)?" +
> +    "(([Zz]|([+-])([0-9]{2}):([0-9]{2})))?"

You can spare one pair of brackets here: "([Zz]|([+-])([0-9]{2}):([0-9]{2}))?"

In that case, you have to lower the match indices greater than 7 in fromRFC3339FixedZone by 1

::: calendar/test/unit/test_gdata_provider.js
@@ +523,5 @@
> +            "BEGIN:VEVENT",
> +            "UID:123",
> +            "DTSTART;TZID=" + tzid + ":20150130T120000",
> +            "END:VEVENT",
> +            "END:VCALENDAR"].join("\r\n");

You have no RRULEs in the vtimzone definition, but you probably want to. The tests below eventually only work, because you're using a standard time date and STD time is defined to start later than DST for that timezone and so everything is STD time.

@@ -523,4 @@
>  
>      // valid continent/region/city Olson tz
> -    dt = _createDateTime(tzProvider.getTimezone("America/Argentina/Buenos_Aires"));
> -    equal(dateToJSON(dt), {"dateTime": "2015-01-30T12:00:00", "timeZone": "America/New_York"});

I' wondering why the test didn't fail before, when the event was converted to BA timezone and the check was expecting to get a NY timezone instead. Does that test work at all?

@@ +554,2 @@
>  
> +    // ical.js and libical currently have slightly different timezone handling.

You're working around here a lack in ical.js tz handling. Treating an unknown timzone as floating instead of applying the vtimetone component here is imo not the intended behaviour. Does already a bug exist to make it working like libical does (I think so, but not I'm not sure)? If not, can file one?

@@ +606,5 @@
> +    // A date, with a timezone that has zero offset
> +    equal(convert({ "date": "2015-01-02", "timeZone": "Africa/Accra" }), "20150102 in Africa/Accra");
> +
> +    // A date, using a timezone with a nonzero offset that is not the default timezone
> +    equal(convert({ "date": "2015-01-02", "timeZone": "Asia/Baku" }), "20150102 in Asia/Baku");

Don't you want to check for the default timezone here first and apply one of two candidates based on the result? It's unlikely that someone with default timezone Asia/Baku runs the test, but if so a failure eventually wouldn't get obvious.

@@ +622,5 @@
> +    equal(convert({ "dateTime": "2015-07-01T12:13:14+01:00", "timeZone": "Europe/London" }, "Europe/London"), "20150701T121314 in Europe/London");
> +
> +    // An event in Los Angeles, with a calendar set to Asia/Baku
> +    equal(convert({ "dateTime": "2015-07-01T12:13:14+05:00", "timeZone": "America/Los_Angeles" }, "Asia/Baku"), "20150701T001314 in America/Los_Angeles");
> +    equal(convert({ "dateTime": "2015-12-01T12:13:14+04:00", "timeZone": "America/Los_Angeles" }, "Asia/Baku"), "20151201T001314 in America/Los_Angeles");

That test does the same as the Berlin test above, so can can leave this out. The only difference is that you pass the tz argument instead of using the default value to convert(...).

@@ +631,5 @@
> +    // An offset matching the passed in calendar timezone. This should NOT be Africa/Algiers
> +    equal(convert({ "dateTime": "2015-01-02T03:04:05+01:00" }), "20150102T030405 in Europe/Berlin");
> +
> +    // An offset that doesn't match the calendar timezone, will use the first timezone in that offset
> +    do_print("The following warning is expected: 2015-01-02T03:04:05+04:00 does not match timezone offset for Europe/Berlin");

Does do_print work? Iirc, it doesn't work for me recently.

Comment 25

[Philipp Kewisch [:Fallen][☁️📆]]

(In reply to MakeMyDay from comment #24)

> 
> You can spare one pair of brackets here:
> "([Zz]|([+-])([0-9]{2}):([0-9]{2}))?"
> 
> In that case, you have to lower the match indices greater than 7 in
> fromRFC3339FixedZone by 1
From testing, it is actually only indices greater 9. The code is mostly copied from calProviderUtils, hence also the != null code. I'll make the changes and make sure the tests pass. It is easy to make mistakes when changing regexes :)


> You have no RRULEs in the vtimzone definition, but you probably want to. The
> tests below eventually only work, because you're using a standard time date
> and STD time is defined to start later than DST for that timezone and so
> everything is STD time.
Good catch, I've added an RRULE from one of our other tests.

> 
> @@ -523,4 @@
> >  
> >      // valid continent/region/city Olson tz
> > -    dt = _createDateTime(tzProvider.getTimezone("America/Argentina/Buenos_Aires"));
> > -    equal(dateToJSON(dt), {"dateTime": "2015-01-30T12:00:00", "timeZone": "America/New_York"});
> 
> I' wondering why the test didn't fail before, when the event was converted
> to BA timezone and the check was expecting to get a NY timezone instead.
> Does that test work at all?
From what I remember, the tests all passed, even with all other tests commented out.


> You're working around here a lack in ical.js tz handling. Treating an
> unknown timzone as floating instead of applying the vtimetone component here
> is imo not the intended behaviour. Does already a bug exist to make it
> working like libical does (I think so, but not I'm not sure)? If not, can
> file one?
I don't think we have one, filed bug 1212914


> 
> @@ +606,5 @@
> > +    // A date, with a timezone that has zero offset
> > +    equal(convert({ "date": "2015-01-02", "timeZone": "Africa/Accra" }), "20150102 in Africa/Accra");
> > +
> > +    // A date, using a timezone with a nonzero offset that is not the default timezone
> > +    equal(convert({ "date": "2015-01-02", "timeZone": "Asia/Baku" }), "20150102 in Asia/Baku");
> 
> Don't you want to check for the default timezone here first and apply one of
> two candidates based on the result? It's unlikely that someone with default
> timezone Asia/Baku runs the test, but if so a failure eventually wouldn't
> get obvious.
I'll double check this, I believe the default timezone is floating when running xpcshell tests.


> That test does the same as the Berlin test above, so can can leave this out.
> The only difference is that you pass the tz argument instead of using the
> default value to convert(...).
Ok

> Does do_print work? Iirc, it doesn't work for me recently.
It has been working for me, but I've also had the case before where it didn't. Not our bug to fix though :)

Comment 26

[Philipp Kewisch [:Fallen][☁️📆]]

(In reply to Philipp Kewisch [:Fallen] from comment #25)
> > You have no RRULEs in the vtimzone definition, but you probably want to. The
> > tests below eventually only work, because you're using a standard time date
> > and STD time is defined to start later than DST for that timezone and so
> > everything is STD time.
> Good catch, I've added an RRULE from one of our other tests.
Adding the rrule doesn't change anything w.r.t. tests passing.

> > 
> > @@ -523,4 @@
> > >  
> > >      // valid continent/region/city Olson tz
> > > -    dt = _createDateTime(tzProvider.getTimezone("America/Argentina/Buenos_Aires"));
> > > -    equal(dateToJSON(dt), {"dateTime": "2015-01-30T12:00:00", "timeZone": "America/New_York"});
> > 
> > I' wondering why the test didn't fail before, when the event was converted
> > to BA timezone and the check was expecting to get a NY timezone instead.
> > Does that test work at all?
> From what I remember, the tests all passed, even with all other tests
> commented out.
I've run the tests with patch applied again and they work. The tests normally don't run correctly because of missing do_test_pending/do_test_finished, but this is bug 1190012 and I'd rather fix there. I've run them locally and made sure all checks are done.


> > @@ +606,5 @@
> > > +    // A date, with a timezone that has zero offset
> > > +    equal(convert({ "date": "2015-01-02", "timeZone": "Africa/Accra" }), "20150102 in Africa/Accra");
> > > +
> > > +    // A date, using a timezone with a nonzero offset that is not the default timezone
> > > +    equal(convert({ "date": "2015-01-02", "timeZone": "Asia/Baku" }), "20150102 in Asia/Baku");
> > 
> > Don't you want to check for the default timezone here first and apply one of
> > two candidates based on the result? It's unlikely that someone with default
> > timezone Asia/Baku runs the test, but if so a failure eventually wouldn't
> > get obvious.
> I'll double check this, I believe the default timezone is floating when
> running xpcshell tests.
Ok, terminology here. The "default timezone" is not the zone where the user is running the test, but hints to the default timezone of the calendar, which is important for how it is called outside of the tests. In this case I'm defining that the default to Europe/Berlin, so the tests should be fine. 

> 
> 
> > That test does the same as the Berlin test above, so can can leave this out.
> > The only difference is that you pass the tz argument instead of using the
> > default value to convert(...).
I'm going to leave this one in, doesn't hurt to have one more set of tests with different timezone offsets.

 
> > Does do_print work? Iirc, it doesn't work for me recently.
> It has been working for me, but I've also had the case before where it
> didn't. Not our bug to fix though :)
Checked this again, do_print works with --verbose, but not without. I think that is ok for now.

Comment 27

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Fix - v5 (for checkin)

Comment 28

[Petr Špaček]

Hmm, it seems that there is a final version of the patch attached to this but. Would it be possible to move this forward? DST change in Europe and US happened during last weeks so my calendar now shows misaligned events right now and that it pretty annoying.

Thank you very much for your time!

Comment 29

[Petr Špaček]

If it is worth, I confirm that manual application of the patch v5 to my local installation fixed the problem.

Comment 30

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/47296fb23b5b3328e0c079b2c58722e195e2c7f1
Bug 1146286 - Events from before daylight savings time change are one hour off. r=MakeMyDay a=aleth CLOSED TREE

Comment 32

[Phil Evans]

Has anyone reported this bug to Google?
My reading of ISO9601 is that the "Z" should only be appended to the date/time if the time is in UTC, i.e. while you folks have created a fix for the lightning plugin (thanks! It works great) - fundamentally, this is a bug in Google calendar.