Closed Bug 1691885 Opened 2 years ago Closed 2 years ago

Calendar notifications are not emitted sequentially

Categories

(Calendar :: Provider: CalDAV, defect)

defect

Tracking

(thunderbird_esr78 wontfix)

RESOLVED FIXED
89 Branch
Tracking Status
thunderbird_esr78 --- wontfix

People

(Reporter: darktrojan, Assigned: darktrojan)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

The CalDAV provider emits onStartBatch, onEndBatch, and onLoad when it's performing network operations. However, the onLoad notification should happen after the others when everything is complete. Currently for calendars without offline support it is happening first, and for calendars with offline support it is not happening at all.

I think this might be what's causing the problems with alarms (bug 1666156 etc.) but I'm not sure because I can't reproduce them at the moment. It certainly is causing some problems with alarms such as requiring dismissal a second time in some circumstances.

In this bug I'm going to make the notifications happen in sequence, and always with onLoad at the end (except in one case where that seems to be wrong and it causes a problem anyway). I'm also going to have alarms for network calendars only fire in response to the onLoad notification to stop onAddItem/onModifyItem causing a double firing.

Hopefully that will make things less prone to problems.

This patch makes the onLoad notification happen at the end of a load (as opposed to the start or not at all)
and has the alarm service respond only to that notification for network calendars.

Even with this patch the onAddItem/onModifyItem notifications usually happen outside of the onStartBatch/onEndBatch notifications for calendars with offline storage. This is because they come from a bunch of spaghetti code that tells the storage back end what to do, and I've not found a way to wait for that to happen before continuing.

(In reply to Geoff Lankow (:darktrojan) from comment #0)

I think this might be what's causing the problems with alarms (bug 1666156 etc.) but I'm not sure because I can't reproduce them at the moment.

Which version of TB are you testing with?
Have you tried with the reproducible steps described in Bug 1666156 Comment 22 ?

Depends on: 1700024
Blocks: 1700047
Attachment #9202263 - Attachment description: Bug 1691885 - Make calendar notifications fire in the right sequence. → Bug 1691885 - Make CalDAV calendar notifications fire in the right sequence. r?Fallen

This tests each combination of calendar provider and offline storage setting to ensure they behave consistently.
To do this I've added mock CalDAV and ICS servers (forked from CardDAVServer.jsm).

Depends on D104651

Attachment #9202263 - Attachment description: Bug 1691885 - Make CalDAV calendar notifications fire in the right sequence. r?Fallen → Bug 1691885 - Make CalDAV calendar notifications fire in the right sequence. r?mkmelin
Attachment #9210666 - Attachment description: Bug 1691885 - Test notifications from CalDAV and ICS calendars. r?Fallen → Bug 1691885 - Test notifications from CalDAV and ICS calendars. r?mkmelin

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/88c4fe2fed10
Make CalDAV calendar notifications fire in the right sequence. r=mkmelin
https://hg.mozilla.org/comm-central/rev/6ac8a49c6e5d
Test notifications from CalDAV and ICS calendars. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED

I'm about to file a follow-up bug for the cached ICS calendar.

Target Milestone: --- → 89 Branch
Blocks: 1702369
Regressions: 1708596
See Also: → 1709055
You need to log in before you can comment on or make changes to this bug.