Closed
Bug 697639
Opened 13 years ago
Closed 12 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)
Calendar
Dialogs
Tracking
(Not tracked)
RESOLVED
FIXED
1.2
People
(Reporter: mmecca, Assigned: mmecca)
Details
Attachments
(2 files, 1 obsolete file)
1.39 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
6.62 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
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•13 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•13 years ago
|
||
Prevents the recurrenceInfo of the original item from being modified by the dialog before the item is updated.
Attachment #574237 -
Flags: review?(philipp)
Comment 3•13 years ago
|
||
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•12 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•12 years ago
|
||
This passes OK for me.
Attachment #580309 -
Flags: review?(philipp)
Comment 6•12 years ago
|
||
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•12 years ago
|
||
Pushed to comm-central - http://hg.mozilla.org/comm-central/rev/c1e60860f7bf
Assignee | ||
Comment 8•12 years ago
|
||
Pushed to comm-aurora - http://hg.mozilla.org/releases/comm-aurora/rev/c17413b85bd1
Assignee | ||
Updated•12 years ago
|
Attachment #574237 -
Attachment description: Fix v1 → [checked-in] Fix v1
Assignee | ||
Comment 9•12 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)
Updated•12 years ago
|
Target Milestone: --- → 1.2
Assignee | ||
Comment 10•12 years ago
|
||
Moves clone tests to test_recur.js
Attachment #580309 -
Attachment is obsolete: true
Attachment #582533 -
Flags: review?(philipp)
Comment 11•12 years ago
|
||
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•12 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•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•