Closed
Bug 605378
Opened 14 years ago
Closed 12 years ago
Fail to parse supported-calendar-component-set results from Chandler PROPFIND
Categories
(Calendar :: Provider: CalDAV, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.2
People
(Reporter: glen.a.ritchie, Assigned: sfleiter)
References
Details
(Whiteboard: [CalDAV server: Chandler])
Attachments
(1 file, 1 obsolete file)
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.2.9) Gecko/20100915 Lightning/1.0b3pre Thunderbird/3.1.4 Recently I had issues after the upgrade to lightning 1.02b and Thunderbird 3.1.4 which broke CalDAV calendar support while using Chandler(Cosmos) server. Specifically I was unable to create events on the calendar, as the calender was simply not listed in the available calendars when trying to create an event. Reproducible: Always Steps to Reproduce: 1. Add network calendar from Chandler CalDAV 2. Attempt to create event on new calendar Actual Results: Unable to create event on the new calendar Expected Results: Should be able to create events on the calendar - Chandler Server 1.1.0. While investigating the cause, I determined that the CalDAV module was not parsing the PROPFIND results correctly. Chandler was returning(among others) XML like the following string: <C:comp C:name="VEVENT" xmlns:C="urn:ietf:params:xml:ns:caldav" xmlns:D="DAV:"/> The code in calDavCalendar.js attempts to parse ( http://mxr.mozilla.org/comm-1.9.2/source/calendar/providers/caldav/calDavCalendar.js#1388 ) the XML but fails to retrieve the "name" attribute due to the attribute having a namespace associated with it and as a result, believes the server is unable to accept events. Changing the line 1393: let comp = sc.@name.toString(); to let comp = sc.@*::name.toString(); Fixes the issue by allowing it to retrieve the "name" attribute no matter what the namespace. I freely admit my lack of understanding of XML and namespaces, so please feel free to make a better fix instead of my workaround.
Reporter | ||
Updated•14 years ago
|
Version: unspecified → Lightning 1.0b2
Comment 1•14 years ago
|
||
That response returned by Chandler seems OK, even if I am not sure that the attributes really "inherits" the CALDAV: namespace from the xml element "C:comp" Chandler server would probably have much better interoperability if they tried to match their responses with the examples in rfc 4791 (i.e.: never specify the namespace on attributes). That being said I agree with the filer's recommended fix.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•13 years ago
|
Whiteboard: [CalDAV server: Chandler]
Updated•13 years ago
|
OS: Windows XP → All
Hardware: x86 → All
Assignee | ||
Comment 3•13 years ago
|
||
I repeat the most important points from duplicate bug 707231 I created: The XML Chandler/Cosmo does create is right. At least according to "xmllint --debug" which dumps a debug tree of the in-memory document the C namespace prefix for the attribute is valid but redundant, so both kinds of writing the "name" attribute should work. If sc is in the namespace C, shouldn't sc.@name match to name attributes in namespace C? C:name and name are equivalent so the parsing code should handle that. My guess is that sc.@*::name.toString(); matches to name attributes in any namespace which would be wrong. Where do I find the documentation to the XML parsing done here: let comp = sc.@name.toString(); ? I would very much like to understand that code. Thanks in advance.
Comment 4•13 years ago
|
||
The sc.@name.toString() thing is the javascript xml extensions called "e4x". You could try setting: default xml namespace = C just under the definition of the namespace in that function. I assume that its not using the right default namespace. If that doesn't work, instead of accepting any namespace, I'd suggest using sc.@C::name because the attribute should be from the CalDAV namespace and not from some other namespace. Stefan (Fleiter) I'd appreciate if you could put together a patch, as you seem to have a good grip of the code! A patch for that file would be a start, ideally you could clone http://hg.mozilla.org/comm-central and create a hg diff --git there.
Assignee | ||
Comment 5•13 years ago
|
||
Thank you Philipp for your support, the word e4x was a good starting point. This is very much appreciated. I tried what you suggested, here the results: - Declaring the default namespace does not change anything: http://jsfiddle.net/Stefan_Fleiter/9VeXS/ - using sc.@C::name does not match on attributes which do not have the namespace declared themselves which is quite a surprise. http://jsfiddle.net/Stefan_Fleiter/9VeXS/2/ I will continue with this as time permits.
Assignee | ||
Comment 6•13 years ago
|
||
I had a look at the XML Namespace spec: According to http://www.w3.org/TR/xml-names/#defaulting "The namespace name for an unprefixed attribute name always has no value." That means that an attribute does *not* inherit the namespace of its element. A good description of this fact and the reasons for it can be found here: http://www.xmlplease.com/attributexmlns So the output of "xmllint --debug" was misleading.
Assignee | ||
Comment 7•13 years ago
|
||
So that is a bug in cosmo/chandler and *not* in Lightning. As tested now the cosmo/chandler cloud instance hub.chandlerproject.org does not have this bug anymore. The latest source release of cosmo still has the problem, though. See http://websvn.osafoundation.org/filedetails.php?repname=server&path=%2Fcosmo%2Ftrunk%2Fcosmo%2Fsrc%2Fmain%2Fjava%2Forg%2Fosaf%2Fcosmo%2Fdav%2Fcaldav%2Fproperty%2FSupportedCalendarComponentSet.java and especially the code DomUtil.setAttribute(e, ATTR_CALDAV_NAME, NAMESPACE_CALDAV, type); Since cosmo, the ClaDAV server of chandler, is a dead project I do not think that this will be fixed in the source in the future. I do not know how many companies/organizations have cosmo servers running or will setup new ones and whether a workaround should be implemented. If no workaround is wanted this should be closed as INVALID. If a workaround would be accepted I could create one.
Assignee | ||
Comment 8•13 years ago
|
||
Sorry for bugspam, last comment for today: A fix could look like the fallback logic here: http://jsfiddle.net/Stefan_Fleiter/9VeXS/3/
Comment 9•13 years ago
|
||
Ok, since @C::name doesn't work I think its ok to just workaround using @*::name, adding a comment that this is needed for cosmo. Are there any other places you know of where cosmo adds a namespace to the attribute? If so we should add workarounds there too. Thank you for your detailed analysis
Assignee | ||
Comment 10•13 years ago
|
||
That would be http://jsfiddle.net/Stefan_Fleiter/9VeXS/4/ Especially interesting is the FOO_BAR calendar component type. ;-) No I do not know (yet) any other place where cosmo uses a namespaced attribute. If I find one I will report it here.
Comment 11•13 years ago
|
||
Yep, lets go with that version. Could you attach a patch to this bug and ask me for review?
Assignee | ||
Comment 12•13 years ago
|
||
Will do that during the next days.
Assignee | ||
Comment 13•13 years ago
|
||
Attachment #581066 -
Flags: review?(philipp)
Assignee | ||
Comment 14•13 years ago
|
||
This patch does not help until bug 588799 is fixed, too. So that should be next. :-)
Assignee | ||
Comment 15•13 years ago
|
||
Comment on attachment 581066 [details] [diff] [review] workaround for wrong namespace of name attribute in comp element of supported-calendar-component-set reponse Having slept a night over this the comment is quite misleading, sorry. I will generate a better patch when I am at my private computer again.
Attachment #581066 -
Flags: review?(philipp)
Assignee | ||
Comment 16•13 years ago
|
||
This time with better (not misleading) comment.
Attachment #581066 -
Attachment is obsolete: true
Attachment #581352 -
Flags: review?(philipp)
Updated•13 years ago
|
Assignee: nobody → stefan.fleiter
Status: NEW → ASSIGNED
Comment 17•12 years ago
|
||
Comment on attachment 581352 [details] [diff] [review] workaround for wrong namespace of name attribute in comp element of supported-calendar-component-set reponse (better comment) Review of attachment 581352 [details] [diff] [review]: ----------------------------------------------------------------- r=philipp and approval for comm-aurora since its low risk ::: calendar/providers/caldav/calDavCalendar.js @@ +1609,5 @@ > if (supportedComponentsXml.C::comp.length() > 0) { > thisCalendar.mSupportedItemTypes.length = 0; > for each (let sc in supportedComponentsXml.C::comp) { > + // accept name attribute from all namespaces to workaround Cosmo bug > + // see https://bugzilla.mozilla.org/show_bug.cgi?id=605378#c6 I think it sufficient to write bug 605378 comment 6, as we've done this the same in other places. I'll fix this before checkin.
Attachment #581352 -
Flags: review?(philipp) → review+
Comment 18•12 years ago
|
||
Pushed to: comm-central changeset 55b341cdcf82 releases/comm-aurora changeset 0274ebc20d7f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in
before you can comment on or make changes to this bug.
Description
•