349715 - Error: "alarmTime has no properties" on tasks created by KDE Kontact

Status: VERIFIED FIXED

(Calendar :: Alarms, defect)

Description

[andrewz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6

A certain iCal item causes a JavaScript error.

Reproducible: Always

Steps to Reproduce:
1. Delete the Sunbird data directory.
2. Either "File->Import" or "File->Subscribe to remote calendar" to the attached .ics.

Actual Results:  
Error: uncaught exception: [Exception... "'[JavaScript Error: "alarmTime has no properties" {file: "file:///C:/Documents%20and%20Settings/andrewz/Desktop/sunbird-0.3a2+.en-US.win32/sunbird/js/calAlarmService.js" line: 348}]' when calling method: [calIOperationListener::onGetResult]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: file:///C:/Documents%20and%20Settings/andrewz/Desktop/sunbird-0.3a2+.en-US.win32/sunbird/components/calStorageCalendar.js :: queueItems :: line 580"  data: yes]

Expected Results:  
No error messages.

Same error message as issue 336766 (which is fixed), but I verified that the sample file in that issue no longer causes an error.

Comment 1

[andrewz]

Attachment: creates a javascript error... created by KDE Kontact v 1.2.3

Comment 2

[Joey Minta]

Sigh... someone really needs to take the VALARM part of rfc2445 out back and shoot it.

From 4.8.6.3 
  "The default duration is relative to the start of an event or to-do
   that the alarm is associated with. The duration can be explicitly set
   to trigger from either the end or the start of the associated event
   or to-do with the "RELATED" parameter."

The alarm attached here applies to a todo with only a DUE property.  According to my reading of the above, this means the TRIGGER is required to have RELATED:END.  Otherwise, we're to assume that the alarm corresponds to the (non-existent in this case) DTSTART property of the task.

Note that it's not too hard to have our code also handle this interpretation, but I'm not sure I want to go down the road of encouraging deviations from the standard.  In that case, this is a bug in Kontact.

Comment 3

[Joey Minta]

Attachment: patch

For reference, this is that patch that would probably fix this bug.  Untested, but straightforward enough.

Comment 4

[andrewz]

For cross reference, the new KDE bug is http://bugs.kde.org/show_bug.cgi?id=132818

Comment 5

[andrewz]

The patch from comment #4 does not seem to help in this case, but applying a very similar fix near line 348 makes the error message go away.

if (aItem.alarmRelated == Components.interfaces.calIItemBase.ALARM_RELATED_START) {
-            alarmTime = aItem.startDate || aItem.entryDate;
+            alarmTime = aItem.startDate || aItem.entryDate || aItem.dueDate;
        } else {
            alarmTime = aItem.endDate || aItem.dueDate;
        }

Comment 6

[Joey Minta]

(In reply to comment #5)
> The patch from comment #4 does not seem to help in this case, but applying a
> very similar fix near line 348 makes the error message go away.
Oh, oops.  You need both, otherwise snoozing that alarm wouldn't work. :-)

Comment 7

[Stefan Sitter [:ssitter]]

Updating summary as this is caused by a non-standard *.ics produced by KDE Kontact.
Open Question: Should Calendar follow the standard or or make Calendar less strict and accept that *.ics?

Comment 8

[Stefan Sitter [:ssitter]]

Attachment: make alarm handling less strict, fallback to date / due date

This hopefully helps on Bug 351977, Bug 356000 and Bug 356004 too.

Comment 9

[Stefan Sitter [:ssitter]]

*** Bug 351977 has been marked as a duplicate of this bug. ***

Comment 10

[Stefan Sitter [:ssitter]]

*** Bug 356004 has been marked as a duplicate of this bug. ***

Comment 11

[Matthew (lilmatt) Willis]

Comment on attachment 241841 [details] [diff] [review]
make alarm handling less strict, fallback to date / due date

r=lilmatt

Comment 12

[Matthew (lilmatt) Willis]

Comment on attachment 241841 [details] [diff] [review]
make alarm handling less strict, fallback to date / due date

Moving r2 to ctalbert per dmose

Comment 13

[cmtalbert]

Comment on attachment 241841 [details] [diff] [review]
make alarm handling less strict, fallback to date / due date

Looks good. As a side note, we should figure out how to best drive clarification on these pieces of RFC 2445.

Comment 14

[Matthew (lilmatt) Willis]

Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED

Comment 15

[Matthew (lilmatt) Willis]

(In reply to comment #13)
> As a side note, we should figure out how to best drive
> clarification on these pieces of RFC 2445.

calsify

Comment 16

[Sebastian Schwieger]

The attached ics file does not result in error message any more. It fires an alarm. (I changed the due date to 1st of May 2006 in order to get the alarm) 
-> Verified fixed

Comment 17

[Dan Mosedale (:dmosedale, :dmose)]

Taking for 0.3.1 because it also gets trigged by old Sunbird/Lightning alarms, not just KDE alarms (see bug 356004).

Comment 18

[Matthew (lilmatt) Willis]

Patch checked in for 0.3.1

LIGHTNING_0_3_BRANCH:
Checking in calAlarmService.js;
/cvsroot/mozilla/calendar/base/src/calAlarmService.js,v  <--  calAlarmService.js
new revision: 1.8.2.13.2.1; previous revision: 1.8.2.13

SUNBIRD_0_3_BRANCH:
Checking in calAlarmService.js;
/cvsroot/mozilla/calendar/base/src/calAlarmService.js,v  <--  calAlarmService.js
new revision: 1.22.2.1; previous revision: 1.22

Marking [fixed0.3.1]