393816 - X-MOZ-LOCATIONPATH is pointless

Status: RESOLVED FIXED

(Calendar :: Provider: CalDAV, defect)

Description

[Bruno Browning]

Since CalDAV servers can (and do) save calendar items both in the calendar URI and in its subcollections, the CalDAV provider needs to track the server-local path to calendar items. Currently, this path is getting serialized to the item's icalString as an X-MOZ-LOCATIONPATH property. We also reset the locationpath of an item every time it is REPORTed, and thus never actually use the serialized property. Since we don't need it, we should eliminate it.

Taking this. Patch in hand which depends on code in the patch for bug 373370, so I'm setting that as a blocker for this bug.

Comment 1

[Bruno Browning]

Attachment: eliminate x-prop

not requesting review yet pending resolution of bug 373370.
portions of this patch originally by [e-mail]

Comment 2

[Bruno Browning]

Attachment: de-bitrot patch

de-bitrotting and requesting review, not from dbo since some of this patch was his originally. Not a terribly important bug in itself, but part of this patch will be needed for fixing 393817, which I think is more important.

Comment 3

[cmtalbert]

Comment on attachment 288942 [details] [diff] [review]
de-bitrot patch

>Index: calendar/providers/caldav/calDavCalendar.js
>===================================================================

>     refresh: function caldav_refresh() {
>-        // clear etag cache
>-        this.mEtagCache = [];
>         this.mObservers.notify("onLoad", [this]);
>     },
I don't understand this part.  Why do you NOT want to clear the EtagCache when you refresh?  It seems like on Refresh you want to grab a new selection of data, so you'd clear that too.  Perhaps I'm missing something.


Otherwise, this looks fine.  I'm sorry it took so long to get to this review, because this patch has terribly bit-rotted.  We'll need a new patch and an explanation of the above, but I think that's pretty trivial. So, I'll say this looks good.

r=ctalbert pending the question above...

Comment 4

[Bruno Browning]

Attachment: de-bitrot, clear cache

Patch with bitrot cleaned up and the cache clearing re-added. We really do want to stop clearing it, but that won't really be needed until my next patch, so that's where we'll do it.

Comment 5

[Bruno Browning]

Checked in on HEAD and MOZILLA_1_8_BRANCH.