383462 - VTODO with RRULE but no DTSTART causes exception, missing entries

Status: VERIFIED FIXED

Description

[Daniel Boelzle [:dbo]]

Created attachment 267416 [details]
sample

Invalid pointer exception in calRecurrenceRule::GetOccurrences.

DTSTART is optional for VTODOs, RFC 2445 is IMO unclear whether RRULE is allowed in that case. We should fallback using DUE if no DTSTART is present, expanding the rule. IMO that has been the intention of such VTODOs.

Comment 1

[Daniel Boelzle [:dbo]]

Created attachment 267417 [details] [diff] [review]
fallback to DUE if no DTSTART is present

Comment 2

[Stefan Sitter]

Adding the error messages as shown in console to make it possible to search and reference them:

Error: Import error: [Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [calIRecurrenceItem.getOccurrences]"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame :: file:///D:/sunbird/js/calRecurrenceInfo.js :: anonymous :: line 432"  data: no]
Source file: chrome://calendar/content/import-export.js
Line: 196

Error: Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [calIRecurrenceItem.getOccurrences]
Source file: file:///D:/sunbird/js/calRecurrenceInfo.js
Line: 432

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 267417 [details] [diff] [review]
fallback to DUE if no DTSTART is present

r=mvl

Comment 4

[Daniel Boelzle [:dbo]]

Checked in on HEAD and MOZILLA_1_8_BRANCH.

Comment 5

[Daniel Boelzle [:dbo]]

Unfortunately bug 386502 has caused a regression:

Error: [Exception... "Component returned failure code: 0x80004003 
(NS_ERROR_INVALID_POINTER) [calIDateTime.compare]" nsresult: "0x80004003 
(NS_ERROR_INVALID_POINTER)" location: "JS frame :: 
... 
:: checkRange :: line 372" data: no]
Source File: 
... calRecurrenceInfo.js
Line: 372

Comment 6

[Daniel Boelzle [:dbo]]

Created attachment 275745 [details] [diff] [review]
fallback to DUE if no DTSTART is present

- fallback to DUE if no DTSTART available, make either one mandatory
- checkRange now might also return DUE: I've renamed variable dtstart to occDate

Comment 7

[Sebastian Schwieger]

The patch looks fine. A comment in the code regarding the fallback might be nice. 

Just two more comments:
1. I wonder if there are other places in the file that need to be changed to fully support recurring task with no start date set. A quick search revealed two more places where item.getProperty("DTSTART") is used. I think you know better than me if they require additional care.

2. Month and MultiWeek View do not support this fallback. A follow up should be filed (if it not yet exists).

Sorry for having caused this regression :(

Comment 8

[Daniel Boelzle [:dbo]]

(In reply to comment #7)
> 1. I wonder if there are other places in the file that need to be changed to
> fully support recurring task with no start date set. A quick search revealed
> two more places where item.getProperty("DTSTART") is used. I think you know
> better than me if they require additional care.
I filed follow-up bug 391495 for that.

> 2. Month and MultiWeek View do not support this fallback. A follow up should be
> filed (if it not yet exists).
Could you please research that?

> Sorry for having caused this regression :(
No problem at all; I've reviewed your patch ;)
Thanks to Andreas finding this during bug verification.

Comment 9

[Daniel Boelzle [:dbo]]

Checked in on HEAD and MOZILLA_1_8_BRANCH.

Comment 10

[Andreas Treumann]

Verified in Lightning and Sunbird (20070809 builds) -> task is fixed.

Comment 11

[Sebastian Schwieger]

(In reply to comment #8)
> Could you please research that?
Bug 349529 is already filed for this. Actually with this fix, the only task-with-only-due-date-set that shows up in the views is the parent item of a recurring event (and probably exceptions thereof).