452214 - CalDAV provider interop issues with newer CalendarServers

Status: RESOLVED FIXED

(Calendar :: Provider: CalDAV, defect)

Description

[Bruno Browning]

Attachment: find and use <owner> prop

Apple has made some changes in newer versions of its CalendarServer (in Mac OS 10.5.3 and higher) that introduce incompatibilities with our bootstrapping/discovery code. Fortunately they've also added a <D:owner> property to calendar resources that we can use to make bootstrapping both faster and more reliable if we find it. This patch does that.

Tested against current and older CalendarServer, current DAViCal and Bedework, and working. Also tested against SOGo, which it breaks since SOGo apparently does not yet support this particular PROPFIND against the principal-URL. So, WIP patch for now, but I'd really like to see this in 0.9: we don't want to release without good Apple interop.

I haven't tested against Google, but if Google supports both the <owner> prop and the principal-URL PROPFIND we may be able to remove some of the Google special-casing.

Comment 1

[Bruno Browning]

Google does provide the <owner> property, so we could remove some of the special-casing. But Google does not support OPTIONS, so we'd need to leave some of the special-casing in - and if we're going to do all that we might as well leave it all in and save a query. Let's hope they fix the OPTIONS bit soon.

Comment 2

[Bruno Browning]

Attachment: change Google specialcasing, add specialcasing for SOGo

This changes the Google specialcasing to make Google follow the same codepath at other servers, and also adds specialcasing for a SOGo bug. I don't know how quickly the SOGo bug will be fixed, nor whether this workaround will work with the SOGo version that just went RC, so I'm not requesting review yet. Comments welcome.

Comment 3

[Simon Paquet [:sipaq]]

What is the status here? How can we proceed? We really want to pack things up code-wise this week for 0.9 RC1.

Comment 4

[Bruno Browning]

(In reply to comment #3)
> What is the status here? How can we proceed? We really want to pack things up
> code-wise this week for 0.9 RC1.

I think that the patch is okay as-is (though I'd probably remove the word "icky"). From irc conversation with Wolfgang I believe that the issue that requires the SOGo workaround is a SOGo bug; I have not been able to test that we still work with their new RC version using this patch, but since the workaround is essentially to ignore the SOGo <owner> prop and revert to previous behavior it should be fine. Will request review from Fallen since he did the original Google workaround.

Comment 5

[Wolfgang Sourdeau]

(In reply to comment #4)

Bruno,


You can remove that work-around. I will fix SOGo so that it behaves properly.
The bug is still present in RC8 but as I will fix my patch for the CalDAV caching, I will also fix that in SOGo trunk.

Comment 6

[Bruno Browning]

(In reply to comment #5)
> You can remove that work-around. I will fix SOGo so that it behaves properly.
> The bug is still present in RC8 but as I will fix my patch for the CalDAV
> caching, I will also fix that in SOGo trunk.

Thanks, Wolfgang. If we remove the workaround, will that not break interop with your installed base? I wonder if we wouldn't be better changing the comment to read that it's for purposes of interop with older versions of SOGo and can be removed when SOGo version ?.? is a fond memory.

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 335867 [details] [diff] [review]
change Google specialcasing, add specialcasing for SOGo

>+        var queryXml;
>+        if (this.mPrincipalUrl) {
...

>+        if (this.mPrincipalUrl) {
>+            httpchannel.requestMethod = "PROPFIND";
>+            httpchannel.setRequestHeader("Depth", "0", false);
I think it would be more clear if you swap the code lines around a bit so that setting the request method is in the same if() block with setting the query xml.


r=philipp codewise

Comment 8

[Bruno Browning]

Attachment: patch against current tree (with wolfgang's 437318 patch)

I'm not sure what the disposition is going to be wrt the patch in 437316, so I'll post the final version of this in two forms, relative to trees both before and after that patch was applied. Patch is debitrotted, addresses Philipp's concerns about clarity, and a milder comment about the SOGo workaround.

Comment 9

[Bruno Browning]

Attachment: patch against tree w/o the (current) 437318 patch

Comment 10

[Philipp Kewisch [:Fallen][📱📆]]

We decided to rather fix the issues from bug 437318 than back out.

Checked in on HEAD and MOZILLA_1_8_BRANCH and SUNBIRD_0_9_BRANCH

-> FIXED