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

RESOLVED FIXED in 3.9

Status

Calendar
ICAL.js Integration
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

(Blocks: 1 bug, {regression})

unspecified
regression
Dependency tree / graph

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
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

3 years ago
Which version of Thunderbird and Lightning do you use? Are there any messages in the log (ctrl+shift+j) when reproducing the issue?
Component: General → ICAL.js Integration

Comment 2

3 years ago
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.
(Assignee)

Comment 3

3 years ago
I've just backed out bug 1063085 locally and this bug went away.
Blocks: 1063085

Comment 4

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

Comment 5

3 years ago
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.
(Assignee)

Comment 6

3 years ago
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.

Updated

3 years ago
Blocks: 978570

Comment 7

3 years ago
Geoff, can you please re-test this with the next Daily build, now that bug 1081534 is fixed?
(Assignee)

Comment 8

3 years ago
Yes, I still see this in 2014-12-09 Daily, with matching Lightning build.
(Assignee)

Comment 9

3 years ago
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.
Assignee: nobody → geoff
Status: NEW → ASSIGNED
Attachment #8541035 - Flags: review?(philipp)
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?
Attachment #8541035 - Flags: review?(philipp) → review+
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.
(Assignee)

Comment 12

2 years ago
(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?
Keywords: checkin-needed
Indeed it would, maybe we should cache this and only switch if the inner zone has changed?
(Assignee)

Updated

2 years ago
Depends on: 1115667
(Assignee)

Updated

2 years ago
No longer depends on: 1115667
(Assignee)

Comment 14

2 years ago
https://hg.mozilla.org/comm-central/rev/0cc5f51af9e5
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 3.9
You need to log in before you can comment on or make changes to this bug.