Closed Bug 697639 Opened 9 years ago Closed 8 years ago

Editing the Start Date of a series of repeating items with an UNTIL date can leave invalid occurrences in the views

Categories

(Calendar :: Dialogs, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mmecca, Assigned: mmecca)

Details

Attachments

(2 files, 1 obsolete file)

Steps to reproduce:

1) Switch to month view
2) Create an event with a custom repeat rule daily, until the next day
3) Verify the 2 occurrences are visible in the view
4) Double-click the item to edit, choose to edit all occurrences
5) Set the start date to 1 hour earlier


Actual results:

The 2 occurrences are shown at the new start time, with a third invalid occurrence shown starting at the last occurrence of the previous start time. Attempting to delete the invalid occurrence results in a MODIFICATION_ERROR


Expected results:

Only the 2 occurrences will be shown at the new start time.
The problem seems to be caused by the way the edit dialog handles the recurrenceInfo
Assignee: nobody → matthew.mecca
Status: NEW → ASSIGNED
Attached patch [checked-in] Fix v1 β€” β€” Splinter Review
Prevents the recurrenceInfo of the original item from being modified by the dialog before the item is updated.
Attachment #574237 - Flags: review?(philipp)
Comment on attachment 574237 [details] [diff] [review]
[checked-in] Fix v1

Looking at

http://mxr.mozilla.org/comm-central/source/calendar/base/src/calRecurrenceRule.cpp#83

it seems like some properties, especially the until date, are not copied while cloning. I've noticed this while working on bug 577777, but I think you should doublecheck that the code that uses window.recurrenceInfo still works properly after cloning.

As an added bonus, you could find out why it wasn't cloned and if there is no good reason then change the C++ code to do so.

I'll reconsider review if you find out this is a non-issue.
Attachment #574237 - Flags: review?(philipp) → review-
(In reply to Philipp Kewisch [:Fallen] from comment #3)
> it seems like some properties, especially the until date, are not copied
> while cloning. 

It looks like it should - the until date is stored in icalrecurrencetype 
http://mxr.mozilla.org/comm-central/source/calendar/libical/src/libical/icalrecur.h#138

and should be copied over here
http://mxr.mozilla.org/comm-central/source/calendar/base/src/calRecurrenceRule.cpp#88
Attached patch Unit tests for calRecurrenceInfo clone (obsolete) β€” β€” Splinter Review
This passes OK for me.
Attachment #580309 - Flags: review?(philipp)
Comment on attachment 574237 [details] [diff] [review]
[checked-in] Fix v1

In that case r+ on this patch. Maybe you can add the clone tests to the test_recur() function in test/unit/test_recur.js instead, this way we have more complete coverage if a new testcase ics comes around.

Approval for aurora too
Attachment #574237 - Flags: review- → review+
Attachment #574237 - Attachment description: Fix v1 → [checked-in] Fix v1
Comment on attachment 580309 [details] [diff] [review]
Unit tests for calRecurrenceInfo clone

I'll post an updated patch for the unit tests in a bit
Attachment #580309 - Flags: review?(philipp)
Target Milestone: --- → 1.2
Moves clone tests to test_recur.js
Attachment #580309 - Attachment is obsolete: true
Attachment #582533 - Flags: review?(philipp)
Comment on attachment 582533 [details] [diff] [review]
Unit tests for calRecurrenceInfo clone v2

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

r=philipp
Attachment #582533 - Flags: review?(philipp) → review+
Comment on attachment 582533 [details] [diff] [review]
Unit tests for calRecurrenceInfo clone v2

Pushed to comm-central - http://hg.mozilla.org/comm-central/rev/3c900f7b83d8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.