Many errors when setting up zimbra calendar

RESOLVED FIXED in 1.0b1

Status

defect
P1
normal
RESOLVED FIXED
11 years ago
8 years ago

People

(Reporter: davida, Assigned: dbo)

Tracking

({dogfood})

Dependency tree / graph
Bug Flags:
tb-integration +

Details

Attachments

(1 attachment, 1 obsolete attachment)

I just built lightning & tb from trunk.

I setup my zimbra caldav calendar.

1) it asked me for my credentials twice.  AFAIK i entered them correctly the first time, but I don't know that for a fact.  Having a real error message if they were incorrect would be good.

2) I got 30 reminders for past events (somehow we should not bring up reminders on first load of calendar, ISTM).  I hit the dismiss all button, and got:

 Warning: There has been an error reading data for calendar: Zimbra.  However, this error is believed to be minor, so the program will attempt to continue. Error code: DAV_PUT_ERROR. Description: There was an error storing the item on the server.

on console (30 times), and an error dialog, whose details button doesn't properly expand the dialog.  

I'm happy to provide more data if that helps figure out the errors.
If you don't already have them, two boolean (true) prefs calendar.debug.log and calendar.debug.log.verbose will get a good bit of diagnostic information spewed to the error console. It sounds like the write to the server to dismiss the alarm is failing; that'll tell us for sure.
I'm seeing a lot of the following in the logs w/ those prefs enabled:

X-LIC-ERROR;X-LIC-ERRORTYPE=VALUE-PARSE-ERROR:No value for LOCATION property
 . Removing entire property:
(In reply to comment #2)
> I'm seeing a lot of the following in the logs w/ those prefs enabled:
> 
> X-LIC-ERROR;X-LIC-ERRORTYPE=VALUE-PARSE-ERROR:No value for LOCATION property
>  . Removing entire property:

This should "go away" with the v3 patch of bug 438964. I'm not sure if X-LIC-ERROR properties are auto-removed by libical, but the empty LOCATION property should now no longer be written.
Google seems to send out empty LOCATION properties, too, on iTIP/iMIP. I don't read anything in the RFC against doing so, thus I think the following libical code needs fixing: <http://mxr.mozilla.org/comm-central/source/calendar/libical/src/libical/icalparser.c#1009>.
However, I don't think this parsing error is related to the Zimbra problem. It only concerns empty properties.
Flags: tb-integration+
Keywords: dogfood
Priority: -- → P2
I'm bumping this to P1, because it essentially means that neither davida nor I can dogfood CalDAV effectively, and since we need good dogfooding experience to make sound product driving decisions...
Priority: P2 → P1
Posted patch fix - v1 (obsolete) β€” β€” Splinter Review
I am not entirely sure, but to me it seems e4x doesn't handle default namespaces correctly. Zimbra's returned multistatus correctly has all hrefs in "DAV:" namespace:

<multistatus xmlns="DAV:">
  <response>
    <href>/principals/users/dbo/</href>
    <propstat>
      <status>HTTP/1.1 200 OK</status>
      <prop>
        <calendar-home-set xmlns="urn:ietf:params:xml:ns:caldav">
          <href xmlns="DAV:">/dav/dbo/</href>
        </calendar-home-set>
        <calendar-user-address-set xmlns="urn:ietf:params:xml:ns:caldav">
          <href xmlns="DAV:">mailto:dbo@momo-vm-01.sj.mozillamessaging.com</href>
          <href xmlns="DAV:">/principals/users/dbo/</href>
        </calendar-user-address-set>
        <schedule-inbox-URL xmlns="urn:ietf:params:xml:ns:caldav">
          <href xmlns="DAV:">/dav/dbo/Inbox/</href>
        </schedule-inbox-URL>
        <schedule-outbox-URL xmlns="urn:ietf:params:xml:ns:caldav">
          <href xmlns="DAV:">/dav/dbo/Sent/</href>
        </schedule-outbox-URL>
      </prop>
    </propstat>
  </response>
</multistatus>

Iterating the response elements the hrefs come unqualified, lacking their correct namespace ("DAV:") either per default xmlns or prefix, and inherit the wrong default xmlns of their parents (i.e. caldav):

response: <response xmlns="DAV:">
  <href>/principals/users/dbo/</href>
  <propstat>
    <status>HTTP/1.1 200 OK</status>
    <prop>
      <calendar-home-set xmlns="urn:ietf:params:xml:ns:caldav">
        <href>/dav/dbo/</href>
      </calendar-home-set>
      <calendar-user-address-set xmlns="urn:ietf:params:xml:ns:caldav">
        <href>mailto:dbo@momo-vm-01.sj.mozillamessaging.com</href>
        <href>/principals/users/dbo/</href>
      </calendar-user-address-set>
      <schedule-inbox-URL xmlns="urn:ietf:params:xml:ns:caldav">
        <href>/dav/dbo/Inbox/</href>
      </schedule-inbox-URL>
      <schedule-outbox-URL xmlns="urn:ietf:params:xml:ns:caldav">
        <href>/dav/dbo/Sent/</href>
      </schedule-outbox-URL>
    </prop>
  </propstat>
</response>

This patch should take care and consolidates some more mixed ns trial code, but needs further testing. I've tested on Zimbra only.
Assignee: nobody → daniel.boelzle
Status: NEW → ASSIGNED
Attachment #349321 - Flags: review?(philipp)
Blocks: 464344
Blocks: 464133
Comment on attachment 349321 [details] [diff] [review]
fix - v1

>             for each (let response in multistatus.*::response) {
> 
>+                function getSafeHrefs(baseElem, tag) {
>+                    function getSafeElem(baseElem, tag) {
>+                        // tries all mixed ns combinations current servers seem to reply...
>+                        let elem = baseElem..C::[tag];
>+                        if (elem.toString().length) {
>+                            return elem;
>+                        }
>+                        elem = baseElem..D::[tag];
>+                        return (elem.toString().length ? elem : null);
>+                    }
>+                    let elem = getSafeElem(baseElem, tag);
>+                    return (elem ? getSafeElem(elem, "href") : []);
>                 }
I'd prefer defining this funtion outside of the loop, maybe even top level. Aside from that I think we should rather be less restrictive on the namespace. Something like the following should also do:

let responseCHS = response..*::["calendar-home-set"]..*::href[0];


It will probably bork if the server puts in some other elements, like:

<D:multistatus xmlns:MC="urn:mycompany:mystuff">
  <D:response>
    <D:href>abc</D:href>
    <MC:href>top-secret-url-link</MC:href>
  </D:response>
</D:multistatus>

but I don't really think thats probable. If you really want you can get the elements using wildcard operators (but no child index) and then retrieve the preferred element using a function similar to this one:

function getPreferredElement(aXMLList, aPreferedNamespace) {
  if (!aPreferredNamespace) {
    aPreferredNamespace = new Namespace("D", "DAV:");
  }
  for (let tag in aXMLList) {
    if (tag.name().uri == aPreferredNamespace.uri) {
      return tag;
    }
  }
  return aXMLList[0];
}

>-                    if (responseCHS.charAt(responseCHS.toString().length -1) != "/") {
>+                    responseCHS = responseCHS[0].toString();
>+                    if (responseCHS.charAt(responseCHS.length -1) != "/") {
>                         responseCHS += "/";
>                     }
I'd prefer using |responseCHS = responseCHS[0].toString().replace(/([^/])$/, "$1/")| (please test, I usually get the syntax wrong!) and no try/catch block.


r- for now, since I'd rather see the wildcard approach, but I'm sure I can be talked into using a function too.
Attachment #349321 - Flags: review?(philipp) → review-
Posted patch fix - v2 β€” β€” Splinter Review
Cool, I didn't know that e4x supports namespace wildcards :)

I agree with using a wildcard. Even though it allows more malformed server responses, it makes our code leaner and less obscure.
Attachment #349321 - Attachment is obsolete: true
Attachment #349716 - Flags: review?(philipp)
Comment on attachment 349716 [details] [diff] [review]
fix - v2

Looks good, r=philipp
Attachment #349716 - Flags: review?(philipp) → review+
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/394ec192af25>

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
OS: Mac OS X → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → 1.0
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.