372839 - Moving an event via drag&drop changes time when crossing DST boundaries

Status: VERIFIED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[lane]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.9) Gecko/20061221 Fedora/1.5.0.9-1.fc5 Firefox/1.5.0.9
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.3pre) Gecko/20070306 Calendar/0.5pre

When dragging an appointment, in month view, from any date before March 11th, 2007, to a date between March 11th and April 1st, the time for the appointment will be changed to one hour earlier.

Reproducible: Always

Steps to Reproduce:
1. Create an appointment before March 11th, 2007
2. Click on month view
3. Drag appointment to a date between March 11th, and March 31st
Actual Results:  
The time for the appointment will be changed to one hour earlier than originally specified.

Expected Results:  
The time for the appointment should remain the same.

Comment 1

[cmtalbert]

I can confirm this on Mac OS X: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.3pre) Gecko/20070306 Calendar/0.5pre with the latest OS timezone fixes.

Note that this occurs any time you drag an event from Standard time into Daylight Savings time.  He forgot to note that his timezone in the original report was America/Chicago which goes to DST on March 11 of 2007.

If you're using the Europe/Berlin timezone, you can recreate this problem by dragging the event from March 4th to March 27, because Europe/Berlin goes to DST on March 25.

We also have the same behavior (except it adds an hour) when dragging from Daylight Savings time to Standard Time.  For example, in the Europe/Berlin test case, you would drag from October 14 to October 31 (Europe/Berlin leaves DST on October 28).

Comment 2

[Matthew (lilmatt) Willis]

Does this happen in 0.3 as well?  I'd like to determine if it is a regression from the 0.3.1 timezone fixes, or if it's just something we've done forever and haven't noticed until now.

In addition, it would help to know if installing or not installing the OS timezone update has any effect on this.

Comment 3

[lane]

The problem does not occur in the following build:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20061006 Sunbird/0.3

But does occur in:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20070213 Sunbird/0.3.1

Comment 4

[cmtalbert]

(In reply to comment #2)
> Does this happen in 0.3 as well?  I'd like to determine if it is a regression
> from the 0.3.1 timezone fixes, or if it's just something we've done forever and
> haven't noticed until now.
> 
> In addition, it would help to know if installing or not installing the OS
> timezone update has any effect on this.
> 
It was confirmed on Mac OS X *with* OS level timezone DST fixes.  And it was confirmed on timezones that did not change their 2007 DST rules (see my example with Europe/Berlin).  So, I think this is a result of us converting to UTC in our storage database for local calendars.  This may be a reason why we should store events in their original timezone, and then display them in whatever timezone the user chooses.

Comment 5

[Matthew (lilmatt) Willis]

This needs to be fixed and should block 0.5.

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

I have found that this definitly has to do with calIDateTime::subtractDate(). When moving an event over the DST border in month view, the duration returned by subtractDate is 23 hours and not 24. This obviously makes the event be one hour off.

subtractDate does some things with libical and such that I currently haven't understood. Maybe I can find out more tomorrow morning, otherwise I hope this helps whoever fixes it.

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: dst shift - v1

Joey reminded me that subtractDate does just the right thing, since strictly there ARE only 23 (25) hours on a DST change. This patch takes care of normalizing the movements to day intervals in the views. I have tested it on each view on both the march and october boundaries (Europe/Berlin). Theoretically I would have expected this doesen't work when moving the event 23 hours or 1 hour in the multiday views, but it seems to works fine.

Comment 8

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: dst shift - v2

Sorry, forgot to update again before I patched. This fixes the bitrot.

Comment 9

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 259569 [details] [diff] [review]
dst shift - v2

>+           if(duration.hours == 23) {
>+             // entering DST
>+             duration.hours++;

>            newStart.addDuration(duration);

So, basically, what you are saying that substract date is correctly returning 23 hours, but addDuration does not work that way, and somehow fails here?
I'd say that the right way is to fix addDuration.

Comment 10

[Philipp Kewisch [:Fallen][📱📆]]

No, both functions are correct. The night of DST is only 23 hours (or 25 hours) long. subtractDate therefore correctly returns either 23 hours (or 1 day and 1 hour). addDuration without this patch then correctly adds 23 hours (or 25 hours).

This patch takes care of adding 24 hours instead of one day with DST applied (which can be 23 or 25 hours long).

Comment 11

[Joey Minta]

Comment on attachment 259569 [details] [diff] [review]
dst shift - v2

Please note in your comment that (for both calls to subtractDate) both arguments are dates (not datetimes) so getting a non-zero number of hours ONLY happens for DST boundary crossings.

+           if(duration.hours == 23) {
+             // entering DST
+             duration.hours++;
+           } else if(duration.hours == 1) {
Spaces between 'f' and '(' here and elsewhere.

r=jminta

Comment 12

[Philipp Kewisch [:Fallen][📱📆]]

Checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Comment 13

[Michiel van Leeuwen (email: mvl+moz@)]

bah, i was hoping that this checkin could wait. I'm not sold on the theory why both substractDate and addDuration are correct, but the code somehow does not work.
As fas as I can see, duration is the time between the original start time and the date of the box the event was dropped on. Then that duration is added to the original start time. How can that not give a correct result?

Comment 14

[Joey Minta]

(In reply to comment #13)
> the original start time. How can that not give a correct result?
> 
Drag-and-drop movements are done through 2 separate calculations.  First, we calculate the number of days (if any) that the drag spanned.  Then we calculate the number of hours/minutes it moved.  The 'bug' here is that when we try to figure out how many days it moved, we actually get 23hrs (or 25hrs), rather than an even number of days.  Then, when we try to adjust the hours, this leaves us off by one, because the hours code assumes the only the date was modified in the first calculation.

In short: subtractDate was being performed on dates, not on the distance actually dragged.

Comment 15

[Omar Bajraszewski]

Verified on Sunbird 0.5 RC1 Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1.4pre) Gecko/20070524 Sunbird/0.5