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

RESOLVED FIXED in 1.2

Status

Calendar
Dialogs
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: mmecca, Assigned: mmecca)

Tracking

unspecified

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

7 years ago
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.
(Assignee)

Comment 1

7 years ago
The problem seems to be caused by the way the edit dialog handles the recurrenceInfo
Assignee: nobody → matthew.mecca
Status: NEW → ASSIGNED
(Assignee)

Comment 2

7 years ago
Created attachment 574237 [details] [diff] [review]
[checked-in] Fix v1

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-
(Assignee)

Comment 4

7 years ago
(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
(Assignee)

Comment 5

7 years ago
Created attachment 580309 [details] [diff] [review]
Unit tests for calRecurrenceInfo clone

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+
(Assignee)

Comment 7

6 years ago
Pushed to comm-central - http://hg.mozilla.org/comm-central/rev/c1e60860f7bf
(Assignee)

Updated

6 years ago
Attachment #574237 - Attachment description: Fix v1 → [checked-in] Fix v1
(Assignee)

Comment 9

6 years ago
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
(Assignee)

Comment 10

6 years ago
Created attachment 582533 [details] [diff] [review]
Unit tests for calRecurrenceInfo clone v2

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+
(Assignee)

Comment 12

6 years ago
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
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.