577023 - The until date of a repeating event changes from UTC time to floating after Thunderbird is closed and restarted.

Status: RESOLVED FIXED

Description

[Decathlon]

This bug is very similar to bug 373667 but happens after Thunderbird is closed and restarted.


Steps to reproduce:
1. create an event with any custom repeating rule and an until date (don't 
   change the default start date-time proposed in the dialog to avoid bug 
   566149)
   e.g.  start date-time 2010/07/05 18:0 
         end date-time 2010/07/05 19:00
         repeat monthly, the 5th of the month, until 2011/07/27
2. save the event;
3. export the calendar in ics format;
4. close and restart Thunderbird;
5. don't modify the event and export again the calendar in ics format.


Actual result:
after step 3 the until date is in UTC format and with a correct value (the same as the start time value but in UTC format) e.g.
   BEGIN:VEVENT
   CREATED:20100705T152858Z
   LAST-MODIFIED:20100705T152917Z
   DTSTAMP:20100705T152917Z
   UID:1aefc9c9-2dfe-4838-8e51-396bbe387f43
   SUMMARY:New Event
   RRULE:FREQ=MONTHLY;UNTIL=20110727T160000Z;BYMONTHDAY=5
   DTSTART;TZID=Europe/Berlin:20100705T180000
   DTEND;TZID=Europe/Berlin:20100705T190000
   END:VEVENT
   
after step 5 the until date is floating but with the same previous date and time, which is wrong as floating e.g.
   RRULE:FREQ=MONTHLY;UNTIL=20110727T160000;BYMONTHDAY=5

Expected result:
until date timezone shouldn't change and should be always in UTC format.


Reproducible: always.

The same occurs with Lightning 0.9

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.8pre) Gecko/20100705 Lightning/1.0b2pre Lanikai/3.1.1pre

Comment 1

[Stefan Sitter [:ssitter]]

This seems specific to storage provider because I can't reproduce using ics provider.

Comment 2

[Decathlon]

Attachment: patch-v1

It seems that the function "newDateTime()" needs the timezone parameter "UTC" for the until date assignment otherwise it returns by default a date with a "floating" timezone. 
This patch seems working fine with anything else to add.

Comment 3

[Markus Adrario [:Taraman]]

Comment on attachment 457320 [details] [diff] [review]
patch-v1

short patch - no further comments.

r=markus

Comment 4

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

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/4019a33839df>
-> FIXED

Comment 5

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

Backported to comm-1.9.2 <http://hg.mozilla.org/releases/comm-1.9.2/rev/33a7cf398f80>

a=philipp

Comment 6

[Decathlon]

I think this bug must be reopened.
I've just seen two events exported from Google calendar and the until date is not always in UTC format.

a normal repeating event:
  DTSTART;TZID=Europe/Paris:20110605T100000
  DTEND;TZID=Europe/Paris:20110605T110000
  RRULE:FREQ=DAILY;UNTIL=20110630T080000Z  <-- UTC
  
an all day repeating event:
  DTSTART;VALUE=DATE:20110605
  DTEND;VALUE=DATE:20110606
  RRULE:FREQ=DAILY;UNTIL=20110630  <-- Floating!


RFC 5545 (http://tools.ietf.org/html/rfc5545#section-3.3.10 pag. 40)
says:

  ...  The value of the UNTIL rule part MUST have the same
  value type as the "DTSTART" property.  Furthermore, if the
  "DTSTART" property is specified as a date with local time, then
  the UNTIL rule part MUST also be specified as a date with local
  time. If the "DTSTART" property is specified as a date with UTC
  time or a date with local time and time zone reference, then the
  UNTIL rule part MUST be specified as a date with UTC time.  In the
  case of the "STANDARD" and "DAYLIGHT" sub-components the UNTIL
  rule part MUST always be specified as a date with UTC time.  If
  specified as a DATE-TIME value, then it MUST be specified in a UTC
  time format.
  
to sum up, the until date should always be UTC except when DSTART is in local time. Stefan, Philipp, is this the right meaning of the text in RFC 5545?

Comment 7

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

Yes, I understand this passage from rfc5545 the same way. So we are doing it wrong?

Comment 8

[Decathlon]

Attachment: [after beta] patch - v2

As far as I can see, the until-date is stored only as native-time format without other informations about the type (isDate) or the timezone (floating or UTC), so I've used the item's start date to select the until-date type.

The patch solves the issue mentioned in comment 6 according to RFC (in particular, when "DTSTART" is date and local time, the same must be the until date). I'm not completely sure if it needs some extra controls to avoid any kind of problem.

Comment 9

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

Comment on attachment 545136 [details] [diff] [review]
[after beta] patch - v2

I think its fine the way it is, but I'd like to push this after 1.0b5 to avoid the risk of last minute regressions. Leaving in my queue so I don't forget.

Comment 10

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

Attachment: Testcase - v1

Not 100% this bug, but I found an error with the until date being saved in UTC. If you subscribe to this calendar as an ics calendar, then the last occurrence of three is not shown since the until date is before the last occurrence (due to timezone differences). I haven't checked if the patch fixes this issue, but maybe it could be taken into account.

Comment 11

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

Comment on attachment 555356 [details]
Testcase - v1

Sorry, I should give credit to jean-louis who originally reported this bug on irc.

Comment 12

[Decathlon]

(In reply to Philipp Kewisch [:Fallen] from comment #10)

> Created attachment 555356 [details]
> Not 100% this bug, but I found an error with the until date being saved in
> UTC. If you subscribe to this calendar as an ics calendar, then the last
> occurrence of three is not shown since the until date is before the last
> occurrence (due to timezone differences). I haven't checked if the patch
> fixes this issue, but maybe it could be taken into account.

Not sure, but it could be bug 566149.
It would need to know how the event has been created. If the user changed the start date to an earlier date/hour before selecting the until date, for sure it's that bug.
See also the duplicate bug 581880 where the timezone difference seemed the cause.

Comment 13

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

Pushed to comm-central changeset 6bb7dbe6d90c

Comment 14

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

Comment on attachment 545136 [details] [diff] [review]
[after beta] patch - v2

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

r=philipp, I'll take care of the push.

::: calendar/providers/storage/calStorageCalendar.js
@@ +1535,5 @@
> +                                let untildate = newDateTime(row.end_date, allday ? "" : "UTC");
> +                                if (allday) {
> +                                    untildate.isDate = true;
> +                                }
> +                                ritem.untilDate = untildate.clone();

No need to clone since you are creating a new date time object a few lines above and not further using it.