Closed Bug 463960 Opened 14 years ago Closed 14 years ago

Cannot refresh cached CalDAV calendars

Categories

(Calendar :: Provider: CalDAV, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: lmarcotte, Assigned: lmarcotte)

Details

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.3) Gecko/2008092414 Firefox/3.0.3
Build Identifier: Lightning 0.9

Using Lightning 0.9, it's not possible to refresh cached CalDAV calendars.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.



In calDavCalendar.js, we have:

    get canRefresh caldav_get_canRefresh() {
      // A cached calendar doesn't need to be refreshed.
      return !this.isCached;
    },

This is wrong as events / tasks can be modified on the server and we need to refresh the cache if that's the case. We should rather have this check:

    get canRefresh caldav_get_canRefresh() {
      if (this.isCached && this.mCheckedServerInfo)
	return true;

      return !this.isCached;
    },

This has been successfully tested with SOGo.
Theoretically, a refresh should be causing the caching wrapper to synchronize and therefore call replayChangesOn(). This is what the comment "A cached calendar doesn't need to be refreshed." meant.
Needs investigation. Ludovic, would you be so kind and propose a trunk patch, too?
Flags: blocking-calendar1.0?
In fact, getRefresh should stay as-is. The problem occurs in safeRefresh.

If the ctag matches, we do not notify our changelog listener. So:

            } else {
                if (thisCalendar.verboseLogging()) {
                    dump("CalDAV: ctag matches, no need to fetch data for calendar "
                        + thisCalendar.name);
                }
		
                // we may still need to poll the inbox
                if (thisCalendar.firstInRealm()) {
                    thisCalendar.pollInBox();
                }

should become:

            } else {
                if (thisCalendar.verboseLogging()) {
                    dump("CalDAV: ctag matches, no need to fetch data for calendar "
                        + thisCalendar.name);
                }
		
		if (thisCalendar.isCached && aChangeLogListener) {
		  aChangeLogListener.onResult({ status: Components.results.NS_OK },
					      Components.results.NS_OK);
		}
		
                // we may still need to poll the inbox
                if (thisCalendar.firstInRealm()) {
                    thisCalendar.pollInBox();
                }
Whiteboard: [needs patch for code in comment#3]
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-calendar1.0? → blocking-calendar1.0+
Also, one thing not to forget (and it'll be included in my upcoming patch) is that if we get an error while fetching the ctag, we should just return like Lightning does right now :

            if (str.substr(0,6) == "<?xml ") {
                    str = str.substring(str.indexOf('<', 2));
            }
            try {
                var multistatus = new XML(str);
            } catch (ex) {
                LOG("CalDAV: Failed to get ctag from server");
                return;
            }

This is wrong as it'll leave pending sync operations in the queue for cached CalDAV calenders. This will prevent Lightning from ever refreshing its cache with the CalDAV server.

To avoid this, we should simply inform the changelog listener, like this:

            if (str.substr(0,6) == "<?xml ") {
                    str = str.substring(str.indexOf('<', 2));
            }
            try {
                var multistatus = new XML(str);
            } catch (ex) {
                LOG("CalDAV: Failed to get ctag from server");
		if (thisCalendar.isCached && aChangeLogListener) {
		  aChangeLogListener.onResult({ status: Components.results.NS_OK },
					      Components.results.NS_OK);
		}
                return;
            }

We don't care, really, about not reporting an error here. Refreshes are frequent enough that we shouln't mark the calendar as readOnly / disabled because one single ctag couldn't be fetched from the server.
Ludovic, we're planning to do a release to accompany TB3 beta2 shortly. Any chance that you could provide a patch for this soon?
Assignee: nobody → lmarcotte
Whiteboard: [needs patch for code in comment#3] → [needs patch for code in comment#3][not needed beta][no l10n impact]
Whiteboard: [needs patch for code in comment#3][not needed beta][no l10n impact] → [not needed beta][no l10n impact][needs patch for code in comment#3]
Issue is still present in the latest nightly build of both Thunderbird and Lightning.
Attached patch Proposed fixSplinter Review
Attachment #363573 - Flags: review?(philipp)
Comment on attachment 363573 [details] [diff] [review]
Proposed fix

I've tested this patch and it seems to work well. I noticed a lot of flashing of events, but that may not be this patches' problem

r=philipp
Attachment #363573 - Flags: review?(philipp) → review+
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/2a165d74fb18>

-> FIXED
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: qawanted
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: [not needed beta][no l10n impact][needs patch for code in comment#3]
Target Milestone: --- → 1.0
(In reply to comment #8)
> (From update of attachment 363573 [details] [diff] [review])
> I've tested this patch and it seems to work well. I noticed a lot of flashing
> of events, but that may not be this patches' problem
> 
> r=philipp

It's not.. but it exposes other problems ;-)  - see my bug report about unnecessary calls to refresh views.
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.