372855 - getOccurrencesBetween() in calTodo.js fails on tasks without entryDate or dueDate

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[Stefan Sitter [:ssitter]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.3pre) Gecko/20070306 Calendar/0.5pre

getOccurrencesBetween() in calTodo.js fails on tasks without entryDate or dueDate. The following error is reported in ensureDateTime() in calUtils.js:

Error: aDate has no properties
Source File: file:///D:/sunbird/js/calUtils.js  Line: 561

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/calendar/base/src/calTodo.js&rev=1.15.2.8&mark=254-258#245

Steps to Reproduce:
1. Start with clean profile
2. Enable View -> Tasks in View
3. Create a new task and set either Start Date or Due Date set
4. Check console and views
5. Edit the task and set both Start Date or Due Date set
6. Check console and views

Actual Results:
After step 4. and 6. the error message from above is shown several times. No task visible in main calendar view.

Expected Results:
No error message. After step 6. the task should be displayed in main calendar view.

Comment 1

[Stefan Sitter [:ssitter]]

Attachment: rev0 - fix regression and views

This removes the regression caused by Bug 353707. Previously convertDate() in calTodo.js used to return null if no argument is passed in. Restoring this behavior fixes getOccurrencesBetween().

After that findColumnsForItem(), doAddEvent() and doDeleteEvent() in multiday-view had to be fixed to work correctly on tasks with only entryDate or dueDate set.

With that patch the task is correctly added/removed to display after creating or editing. Repeating tasks and Drag and Drop also works (except the known issues).

Comment 2

[Stefan Sitter [:ssitter]]

This patch should fix the regression caused by Bug 353707 and should also fix Bug 353066 that is marked blocking 0.5. Therefore also requesting blocking 0.5.

Comment 3

[Matthew (lilmatt) Willis]

blocking 0.5 granted

Comment 4

[Joey Minta]

Comment on attachment 258336 [details] [diff] [review]
rev0 - fix regression and views

-          var startDate = aItem.startDate || aItem.entryDate;
+          var startDate = aItem.startDate || aItem.entryDate || aItem.dueDate;

I don't understand the need for these changes.  onGetResult, onAddItem, onModifyItem, and onDeleteItem all enforce the rule that a task must have both an entryDate and a dueDate.  Can you show me what codepath we're taking where this problem can be hit?

Comment 5

[Stefan Sitter [:ssitter]]

(In reply to comment #4)
The following workflow triggered the error:

- Enable Tasks in View
- Create task with only start date -> task is not shown in view as expected
- Modify task and add due date -> Error: endDate has no properties in findColumnsForItem()

- Enable Tasks in View
- Create task with only due date -> task is not shown in view as expected
- Modify task and add start date -> Error: startDate has no properties in findColumnsForItem()

Comment 6

[Stefan Sitter [:ssitter]]

Attachment: rev1 - fix regression and views

Upon further looking it seems that the real problem is that onModifyItem() removes the old item twice: Once after checking if the item is an event or a task with start and due date and a few lines later again without any check. The latter one called doDeleteEvent() that called findColumnsForItem().

Comment 7

[Joey Minta]

Comment on attachment 259191 [details] [diff] [review]
rev1 - fix regression and views

I like this one much better. :-)
r=jminta

Comment 8

[Michael Büttner (no reviews TFN)]

Checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Comment 9

[Martin Schröder [:mschroeder]]

VERIFIED with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4pre) Gecko/20070522 Calendar/0.5pre.