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

VERIFIED FIXED in Sunbird 0.5

Status

Calendar
Internal Components
VERIFIED FIXED
11 years ago
10 years ago

People

(Reporter: ssitter, Assigned: ssitter)

Tracking

({regression})

Trunk
Sunbird 0.5
regression

Details

(Whiteboard: [patch in hand][no l10n impact])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

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

Updated

11 years ago
Blocks: 353066
(Assignee)

Comment 1

11 years ago
Created attachment 258336 [details] [diff] [review]
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).
Assignee: nobody → ssitter
Status: NEW → ASSIGNED
Attachment #258336 - Flags: first-review?(jminta)
(Assignee)

Comment 2

11 years ago
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.
Flags: blocking-calendar0.5?
Keywords: regression
blocking 0.5 granted
Flags: blocking-calendar0.5? → blocking-calendar0.5+
Whiteboard: [patch in hand] [needs review jminta] [no l10n impact]
Target Milestone: --- → Sunbird 0.5

Comment 4

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

Comment 5

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

Comment 6

11 years ago
Created attachment 259191 [details] [diff] [review]
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().
Attachment #258336 - Attachment is obsolete: true
Attachment #259191 - Flags: first-review?(jminta)
Attachment #258336 - Flags: first-review?(jminta)

Comment 7

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

I like this one much better. :-)
r=jminta
Attachment #259191 - Flags: first-review?(jminta) → first-review+
(Assignee)

Updated

11 years ago
Whiteboard: [patch in hand] [needs review jminta] [no l10n impact] → [patch in hand][no l10n impact][needs checkin]
Checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: [patch in hand][no l10n impact][needs checkin] → [patch in hand][no l10n impact]
VERIFIED with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4pre) Gecko/20070522 Calendar/0.5pre.
Status: RESOLVED → VERIFIED

Updated

10 years ago
Flags: blocking-calendar0.5+
You need to log in before you can comment on or make changes to this bug.