322932 - calEventClass has no properties

Status: RESOLVED WORKSFORME

(Calendar :: Provider: CalDAV, defect)

Description

[Joey Minta]

No good steps to reproduce, but I'm seeing this pretty consistently, especially after switching to a different profile, and then back to a profile that contains a caldav calendar.

Error: calEventClass has no properties
Source File: file:///home/joey/mozhack/sunbird-cc/dist/bin/components/calDavCalendar.js
Line: 535

Comment 1

[Joey Minta]

Attachment: misc changes

Several misc caldav changes:
-remove calEventClass, in favor of immediately parsing the result, and working off of libical's determination of the component type.  This let's us handle returned tasks as well.  (Even though we still can't ask for them.)
-removed several "XXX check [X]" after determining whether or not new changes needed to be made after making such a check.
-updated a few errors to not be NS_ERROR_FAILURE.
-makeOccurrence is obsolete.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 216949 [details] [diff] [review]
misc changes

In general, looks good, but with a few issues:

>@@ -194,17 +182,17 @@ calDavCalendar.prototype = {
> 
>         if (aItem.id == null && aItem.isMutable)
>             // XXX real UUID here!!
>             aItem.id = "uuid" + (new Date()).getTime();
> 
>         if (aItem.id == null) {
>             if (aListener)
>                 aListener.onOperationComplete (this,
>-                                               Components.results.NS_ERROR_FAILURE,
>+                                               Components.results.NS_ERROR_OBJECT_IS_IMMUTABLE,
>                                                aListener.ADD,
>                                                aItem.id,
>                                                "Can't set ID on non-mutable item to addItem");
>             return;
>         }

As long as you're here, can you coalesce above rather curious code flow into the more readable:

if (!aItem.id) {
  if (aItem.isMutable) {
    ...
  } else {
    ...
  }
}

>+            if (aNewItem.isMutable) {
>+                aNewItem.makeImmutable();
>+            }
> 

If isMutable returns true, I think we need to clone it, not have a side effect.

>-                // XXX try-catch
>-                item.icalString = calData;
>+
>+                var icssrv = Components.classes["@mozilla.org/calendar/ics-service;1"]
>+                                       .getService(Components.interfaces.calIICSService);
>+                var calComp = icssrv.parseICS(calData);
>+                var itemComp = calComp.getFirstSubcomponent("ANY");
>+                var item;
>+                if (itemComp.componentType == "VEVENT") {
>+                    item = Components.classes["@mozilla.org/calendar/event;1"]
>+                                     .createInstance(calIEvent);
>+                    item.icalComponent = itemComp;
>+                } else  if (item.componentType == "VTODO") {
>+                    item = Components.classes["@mozilla.org/calendar/todo;1"]
>+                                     .createInstance(calITodo);
>+                    item.icalComponent = itemComp;
>+                } else {
>+                    debug("Got unknown item type:"+item.componentType+'\n');
>+                }
>+                var nextComp = calComp.getNextSubcomponent("ANY");
>+                if (nextComp) {
>+                  //XXX this could be over-ridden instances of the recurring item
>+                  //XXX this could also be a nice timezone that we could use
>+                  Components.utils.reportError(
>+                    "More than one result returned for query\n" +
>+                    " by CalDAV server for URI <" + aResource.spec +
>+                    ">; ignoring");
>+                }
>+

The gist of the above change looks correct, but I think it's going to trigger the crashes mentioned in bug 314334.  So I'd be OK with either making this bug depend on that one, or simply parsing twice for now.

Comment 3

[Joey Minta]

There's some stuff in the patch here that may want to go in at some point (Some of it already has in other bugs.)  The code directly responsible for this bug has been removed.