Last Comment Bug 1082286 - [icaljs] Date/Time Picker seems to have a timezone error
: [icaljs] Date/Time Picker seems to have a timezone error
Status: RESOLVED FIXED
: regression
Product: Calendar
Classification: Client Software
Component: ICAL.js Integration (show other bugs)
: unspecified
: All All
-- normal (vote)
: 3.9
Assigned To: Geoff Lankow (:darktrojan)
:
:
Mentors:
Depends on:
Blocks: icaljs 1063085
  Show dependency treegraph
 
Reported: 2014-10-13 18:47 PDT by Geoff Lankow (:darktrojan)
Modified: 2015-01-01 02:08 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
published.ics (692 bytes, text/plain)
2014-10-26 08:29 PDT, [:MakeMyDay]
no flags Details
1082286-1.diff (1.12 KB, patch)
2014-12-23 17:21 PST, Geoff Lankow (:darktrojan)
philipp: review+
Details | Diff | Splinter Review

Description User image Geoff Lankow (:darktrojan) 2014-10-13 18:47:45 PDT
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 User image [:MakeMyDay] 2014-10-26 05:04:58 PDT
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 User image [:MakeMyDay] 2014-10-26 08:29:16 PDT
Created attachment 8511645 [details]
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 User image Geoff Lankow (:darktrojan) 2014-10-26 16:08:19 PDT
I've just backed out bug 1063085 locally and this bug went away.
Comment 4 User image [:MakeMyDay] 2014-10-27 23:57:10 PDT
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 User image Stefan Sitter 2014-10-28 01:24:46 PDT
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 User image Geoff Lankow (:darktrojan) 2014-10-28 03:32:58 PDT
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 User image [:MakeMyDay] 2014-12-08 23:40:42 PST
Geoff, can you please re-test this with the next Daily build, now that bug 1081534 is fixed?
Comment 8 User image Geoff Lankow (:darktrojan) 2014-12-09 14:19:28 PST
Yes, I still see this in 2014-12-09 Daily, with matching Lightning build.
Comment 9 User image Geoff Lankow (:darktrojan) 2014-12-23 17:21:55 PST
Created attachment 8541035 [details] [diff] [review]
1082286-1.diff

Comparison with cal.floating() always returns false, therefore dateTimeToJsDate doesn't work as expected for floating dates.
Comment 10 User image Philipp Kewisch [:Fallen] 2014-12-24 02:43:52 PST
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 User image Philipp Kewisch [:Fallen] 2014-12-24 02:55:08 PST
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 User image Geoff Lankow (:darktrojan) 2014-12-24 03:42:11 PST
(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 User image Philipp Kewisch [:Fallen] 2014-12-24 05:04:39 PST
Indeed it would, maybe we should cache this and only switch if the inner zone has changed?
Comment 14 User image Geoff Lankow (:darktrojan) 2015-01-01 02:08:49 PST
https://hg.mozilla.org/comm-central/rev/0cc5f51af9e5

Note You need to log in before you can comment on or make changes to this bug.