Closed Bug 697639 Opened 9 years ago Closed 9 years ago
Editing the Start Date of a series of repeating items with an UNTIL date can leave invalid occurrences in the views
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
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
This passes OK for me.
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+
Pushed to comm-central - http://hg.mozilla.org/comm-central/rev/c1e60860f7bf
Pushed to comm-aurora - http://hg.mozilla.org/releases/comm-aurora/rev/c17413b85bd1
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
Moves clone tests to test_recur.js
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: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.