381415 - RDATE/EXDATE list is not considered

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[Daniel Boelzle [:dbo]]

RDATE/EXDATE list is not considered, e.g. the following ics:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN
BEGIN:VEVENT
CREATED:20070520T183929Z
LAST-MODIFIED:20070520T184015Z
DTSTAMP:20070520T183929Z
UID:ed5836b6-43c0-4548-a5e3-65b2bcac0f03
SUMMARY:RDATE sample
RDATE:20070522T100000Z
RDATE:20070523T100000Z
RDATE:20070524T100000Z,20070525T100000Z
DTSTART:20070521T100000Z
DTEND:20070521T110000Z
END:VEVENT
BEGIN:VEVENT
CREATED:20070520T183933Z
LAST-MODIFIED:20070520T184057Z
DTSTAMP:20070520T183933Z
UID:b9cf452c-3b64-4059-ad06-648836e9d5ed
SUMMARY:EXDATE sample
RRULE:FREQ=DAILY;INTERVAL=1
EXDATE:20070522T120000Z
EXDATE:20070523T120000Z
EXDATE:20070524T120000Z,20070525T120000Z
DTSTART:20070521T120000Z
DTEND:20070521T130000Z
END:VEVENT
END:VCALENDAR

Bugs:
=RDATE sample=
- Any RDATE entry removes the original DTSTART instance.
- Comma separated RDATE list isnt' considered, only single ones.
=EXDATE sample=
- Comma separated EXDATE list isn't considered, only single ones.

I am asking myself why we shouldn't consolidate calIRecurrenceDate/calIRecurrenceDateSet. The latter can model both single and multiple RDATEs and EXDATEs. Any objections?

Comment 1

[Dan Cogliano]

I found the same problem with the EXDATE field in 0.5rc1.  Comma separated EXDATE values work OK in 0.3.1 but not in 0.5rc1.  Here is a sample ics that shows the problem:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//ZContent.net//Z Calendar 1.0//EN
CALSCALE:GREGORIAN
METHOD:PUBLISH
BEGIN:VEVENT
SUMMARY:Church Dinner
UID:[e-mail]
SEQUENCE:0
RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=WE;UNTIL=20071231T191500
EXDATE:20070704T191500,20070613T191500,20070620T191500,20070627T191500,200
 70711T191500,20070718T191500,20070725T191500,20070801T191500,20070808T19150
 0,20070815T191500,20070822T191500,20070829T191500
DTSTART:20070103T191500
DURATION:PT45M
CATEGORIES:Church
DESCRIPTION:
END:VEVENT
END:VCALENDAR

Comment 2

[Daniel Boelzle [:dbo]]

Attachment: fixing icalparser

- taking bug
- fixing icalparser to consider comma separated list for RDATE, EXDATE, splitting them up into multiple RDATEs/EXDATEs

Comment 3

[Daniel Boelzle [:dbo]]

Attachment: adding DTSTART for recurring date set

- adds DTSTART to recurring set of dates
- gets rid of aIncludeExceptions parameter (only used with "true")

Comment 4

[Daniel Boelzle [:dbo]]

IMO with the above fixes, we could get rid of calRecurrenceDateSet.*, because parsing will flatten a date list into single RDATEs/EXDATEs.

Comment 5

[Simon Paquet [:sipaq]]

Daniel, is that bug serious enough to take it for 0.5?

Comment 6

[Daniel Boelzle [:dbo]]

No, I don't think so; IMO RDATE/EXDATE lists are quite uncommon. But we should fix it for 0.7, of course...

Comment 7

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

Comment on attachment 268612 [details] [diff] [review]
fixing icalparser

r=mvl, but I suggest yuou move the comment into the comment block that's already about the switch, to make it more clear that all the cases are handled the same.

Comment 8

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

Comment on attachment 268613 [details] [diff] [review]
adding DTSTART for recurring date set

>+        // DTSTART/DUE is always part of the resulting set:
>+        // the base item cannot be replaced by an exception;
>+        // an exception can only be defined on an item resulting from an RDATE/RRULE;
>+        // DTSTART always equals RECURRENCE-ID
>+        dates.push(startDate);

Either I don't understand this comment, or I don't agree with it. rfc2445 says:
   The "EXDATE" property can be used to exclude the value specified in
   "DTSTART". However, in such cases the original "DTSTART" date MUST
   still be maintained by the calendaring and scheduling system because
   the original "DTSTART" value has inherent usage dependencies by other
   properties such as the "RECURRENCE-ID".
I interpret that as saying that you can apply a exdate to the dtstart. And I interpret the comment (and the code) as saying that you cannot do that. Am I missing something?

Comment 9

[Daniel Boelzle [:dbo]]

Yes, I read it the same way. But my comment is related to the (positive) expansion set, meaning this set is always the master item's DTSTART + all positive recurrence items' DTSTARTs. In a second step, the negative recurrence items (EXDATE) filter out dates from that set. "Resulting set" ist misleading here, IMO better the "positive expanded set".

Comment 10

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

Comment on attachment 268613 [details] [diff] [review]
adding DTSTART for recurring date set

ok, I see now. Core to change the comment into what you suggested?

r=mvl

Comment 11

[Daniel Boelzle [:dbo]]

Checked in on HEAD and MOZILLA_1_8_BRANCH.

Comment 12

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

follow-up issue filed for this in bug 386502.

Comment 13

[Andreas Treumann]

Verified in lightning (2007080603) and Sunbird (20070806) -> task is fixed