1082286 - [icaljs] Date/Time Picker seems to have a timezone error

Status: RESOLVED FIXED

(Calendar :: ICAL.js Integration, defect)

Description

[Geoff Lankow (:darktrojan)]

In the Edit Event dialog, the date/time pickers show the time 13 hours in advance of their actual value. (My timezone is UTC+1300.) For example if I click New Event now, it should show 2014-10-14 15:00, but it shows 2014-10-15 04:00.

This occurs with the icaljs backend, but not libical.

Comment 1

[[:MakeMyDay]]

Which version of Thunderbird and Lightning do you use? Are there any messages in the log (ctrl+shift+j) when reproducing the issue?

Comment 2

[[:MakeMyDay]]

Attachment: published.ics

It seems that timezone support is completely broken with icaljs on 3.3.x. For attached pushlished event, with libical timezone offset is correctly applied if you are not in Eastern Standard Time, while icaljs just takes it as local time. Unfortunately, I had not error messages in the log when using icaljs.

Comment 3

[Geoff Lankow (:darktrojan)]

I've just backed out bug 1063085 locally and this bug went away.

Comment 4

[[:MakeMyDay]]

Thanks Geoff for cheking this. Bug 1063085 was alraedy the regression candidate. Can you please also provide your version of Thunderbird and Lightning?

Comment 5

[Stefan Sitter [:ssitter]]

When using Thunderbird builds older than 38.0a1 you might have to set the advanced preference "javascript.options.showInConsole" to "true" to get all error messages reported to console.

Comment 6

[Geoff Lankow (:darktrojan)]

I'm still on Daily from 2014-09-22 due to bug 1081534, and I've just now tested Lightning from the same date; same problem.

Comment 7

[[:MakeMyDay]]

Geoff, can you please re-test this with the next Daily build, now that bug 1081534 is fixed?

Comment 8

[Geoff Lankow (:darktrojan)]

Yes, I still see this in 2014-12-09 Daily, with matching Lightning build.

Comment 9

[Geoff Lankow (:darktrojan)]

Attachment: 1082286-1.diff

Comparison with cal.floating() always returns false, therefore dateTimeToJsDate doesn't work as expected for floating dates.

Comment 10

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8541035 [details] [diff] [review]
1082286-1.diff

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

r+ since it works well, but I wonder why the comparison doesn't work? In theory, there should only be one instance of cal.floating(), so comparing with the timezone property should be working. Either there are multiple globals involved, or the code to hold only one instance is broken. Maybe you can take a quick look into this?

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Ah of course it may be related to that needinfo bug 909183 I have been successfully been procrastinating about for the past 5 months! I'll see if I can finally clear it, every time I looked at it I didn't have a good answer.

Comment 12

[Geoff Lankow (:darktrojan)]

(In reply to Philipp Kewisch [:Fallen] from comment #10)
> r+ since it works well, but I wonder why the comparison doesn't work? In
> theory, there should only be one instance of cal.floating(), so comparing
> with the timezone property should be working. Either there are multiple
> globals involved, or the code to hold only one instance is broken. Maybe you
> can take a quick look into this?

I wondered this too, and I figured it has something to do with this:

> calDateTime.prototype = {
> ...
>     get timezone() new calICALJSTimezone(this.innerObject.zone),

That'd make them different objects and therefore unequal, wouldn't it?

Comment 13

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Indeed it would, maybe we should cache this and only switch if the inner zone has changed?

Comment 14

[Geoff Lankow (:darktrojan)]

https://hg.mozilla.org/comm-central/rev/0cc5f51af9e5