796465 - [calendar] ICAL parser correctly handle timezones

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Calendar, defect, P1)

Description

[GH to BZ]

[GitHub issue by lightsofapollo on 2012-08-09T00:57:36Z, https://github.com/mozilla-b2g/gaia/issues/3279]
Seems like upstream in ICAL.js we do not convert the event. Should happen there or in gaia so free-busy times reflect the current timezone.

Comment 1

[GH to BZ]

[GitHub comment by autonome on 2012-08-20T15:54:45Z]
Re-assign as needed.

Comment 2

[GH to BZ]

[GitHub comment by lightsofapollo on 2012-09-26T08:15:57Z]
db support has landed, ICAL.js can now parse timezone information needs final hookup.

Comment 3

[Dylan Oliver [:doliver]]

Attachment: screenshot - bad timezone

Attaching screenshot of incorrect calendar entries. 

The two long meetings shown in the screenshot ("Review session for basecamp...") are scheduled for 10am Pacific, but the invites were sent from DavidS in Paris and they are showing up in the calendar for 7pm instead (Paris time).

Comment 4

[Dylan Oliver [:doliver]]

This was previous approved blocking+/P2 -- I'm re-nominating for P1 consideration as calendar would become much less useful for anyone working with multiple timezones.

Comment 5

[Borja Salguero [:borjasalguero]]

In Settings and FTU App we have the same issue... We should have a common way of showing the same timezones. What do you think about creating a 'timezones.json' in '/Shared' for having the same list ("GMT +2 Bucharest, Cairo..." for example should be the same in all apps).
This could be useful until having https://bugzilla.mozilla.org/show_bug.cgi?id=714357 fixed.
Could we rename this bug as 'Getting a set of right timezones in Gaia Apps'? Thanks!

Comment 6

[Fabien Cazenave [:kaze]]

We considered using a `shared/resources/` directory to host timezones.json, apns_conf.xml, ringtones, etc. The only difficulty would be related to the build system: in order to know which shared resources should be imported in an app, we should have a reference to it in a <link /> element, like we do for locales.

  <link rel="resource" href="timezones.json" />

If you open a bug like “use a shared/resources/ directory in Gaia”, I’ll happily submit a patch. That being said, I don’t know if that will solve your timezone issues in particular — I’ll trust you on that.

Comment 7

[Borja Salguero [:borjasalguero]]

Great! I've created the following bug https://bugzilla.mozilla.org/show_bug.cgi?id=803029 . Thanks [:kaze] !

Comment 8

[Dietrich Ayala (:dietrich)]

This bug hasn't moved in over a month. What needs to be done to close this?

Comment 9

[James Lal [:lightsofapollo]]

The above discussions don't effect this bug calendar (ical) contains the timezone information we can storage it, parse it and use it but there is a bug in the ical parser. Same deal here P3 = bottom of queue. With the new ical parser design this should be much easier.

This is marked as a "feature" which I believe is incorrect? We simply parse the information sent from the server and use it to offset the position of the event in time relative to the UTC offset in the current timezone.

Comment 11

[Jason Smith [:jsmith]]

Renoming - This was previously a P2, then a P1, now a P3. If we compare the list of bugs for calendar, you'll this clearly does not fit into a P3 bucket because:

Pretty much every P3 is either some usability issue, perf issue, or corner case with reoccurring events. This bug basically basic functionality bustage for timezone-based events - an example I hit was someone in Toronto created an event at 12pm. I see the event at 12pm as well, but that's wrong - it should be 9am.

It also doesn't feel like a P2 either - the P2s cited are about network failures. This is basic bustage with timezones.

I'm arguing P1.

Comment 12

[James Lal [:lightsofapollo]]

Casey, where do we want the UI for timezones? I assume we would want to show which timezone an event is in? For v1 this needs to be read only but we should make it clear in which event it occurs, correct?

Comment 13

[James Lal [:lightsofapollo]]

This has performance impact as well as our recurring rule expansion is very slow. Its possible that we slow sync to a crawl. I am going to do an initial test to see what the relative performance is with the current timezone/recurring expansion implementation.

See bug 817796 for details of optimizing recurring event expansion.

Comment 14

[James Lal [:lightsofapollo]]

My priority is to handle this in the backend, we currently don't have a spot to display this to the user. Thoughts?

Comment 15

[James Lal [:lightsofapollo]]

Progress is being made here. I have an initial working version but the performance of the original recurring event implementation and timezone offset calculations where very slow.

I did a first pass on recurring events (with about 7x speedup there) and now working on timezone offsets. An acceptably fast version (should hopefully speed up overall sync) should be ready tomorrow afternoon (Dec/13)

Comment 16

[James Lal [:lightsofapollo]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7005

Pointer to Github pull-request

Comment 17

[James Lal [:lightsofapollo]]

Comment on attachment 691979 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7005

I volunteered Kevin to review some the underlying ICAL.js patches that make this work. Those must be reviewed independently before we can migrate the code over to gaia. 

Performance on the device has yet to be measured, If possible I want this patch to improve overall sync performance. Failing that we must at least even out.

I have some measurement tools I will use to compare shortly...

Comment 18

[Casey Yee]

I have added a timezone section to the new/edit event screens.

Updated wireframes here:
https://www.dropbox.com/s/31j9d4qrp2v08ny/calendar-events.pdf

Comment 19

[James Lal [:lightsofapollo]]

Hey Casey,

These look good but what about the case where start timezone is different then end timezone? (Start in PST end in EST, etc...).

I am tempted to move the display part out of this bug if we can all agree that is blocking. We should land the back-end functionality ASAP so we can have some people start testing it.

Comment 20

[James Lal [:lightsofapollo]]

Current implementation syncs 32% faster then the previous (in addition to handling timezones). Times measured by the progress indicators presence with marionette.

Each run was executed 3 times (turn on calendar, sync, remove account, sync, etc..).
Each group was executed twice on a clean flash.

I have not yet measured memory improvements but I suspect there are some.

Comment 21

[James Lal [:lightsofapollo]]

One problem remains which is when the timezone is changed calendar needs to restart itself...

Comment 22

[James Lal [:lightsofapollo]]

We now handle the above case and refresh calendar when timezone changes on the device.

We need a followup bug to handle some UI/UX aspects I will post it shortly.

Comment 23

[Jason Smith [:jsmith]]

Verified at a sanity level by using my personal google calendar in the Berlin timezone - events came up at the right time and were listed correctly.