Closed Bug 1182264 Opened 8 years ago Closed 8 years ago

Possible dataloss after editing calendar properties


(Calendar :: General, defect)

Not set


(Not tracked)



(Reporter: mozilla, Assigned: MakeMyDay)


(Keywords: dataloss, regression)


(2 files)

GerryB from reported:
1. Create a new calendar.
2. Create a new event in new calendar.
3. Right-click on new calendar and edit its color.
4. Click OK and OK.

The selection in the calendar list changes and the created event from step 2 disappears. It doesn't come back after restart (dataloss). It seems as if you have to edit the calendar properties once to get a 'healthy' calendar. Afterwards you can create events and edit calendar properties as you like, nothing is lost.
Problem is still present in Lightning 4.4a1.
It seems caused by the patch for bug 768207
Aw man, I thought that was such a simple patch. Can you create a patch for this?
Blocks: ltn402
I'll try.
I'll change the assignment when I'll realize that I am able to fix it. Until then if someone wants to fix, please go on.
Ok, let me know if you need help. I haven't taken a closer look, it sounded like you knew exactly what line is causing it.
This happens only if cache is already enabled for the respective calendar, but caching is not supported. Unfortunately, since bug 768207, caching is enabled anyway - irrespective whether it's supported or not.

This patch takes care to deal with an accidentally enabled cache mode of an calendar and stops setting cache.enabled while creating new calendars, if caching is not supported.

This bug will mainly affect users, who have created their calendars in Lightning 4.0+ and change any calendar property, but there is also a risk for users with previously created calendars, as you can enable/disable cache mode for that calendars by using the property icons in the calendar list. The result in both cases is a complete dataloss for the respective calendar.

That said, I suggest not to wait another 5 weeks for 4.0.2 to get this to lelease, but spin a Lightning immediately to avoid bad user expirience esp. for the new users and bad publicity accordingly.
Assignee: nobody → makemyday
Attachment #8632426 - Flags: review?(philipp)
Severity: normal → critical
Keywords: dataloss, regression
One more thing, Philipp: When checking this, I came accross [1], where REMOVE_UNSUBSCRIBE is used, which is not defined anywhere - should that be REMOVE_NO_DELETE instead? You introduced that with bug 351499.

Comment on attachment 8632426 [details] [diff] [review]

Review of attachment 8632426 [details] [diff] [review]:

(In reply to MakeMyDay from comment #6)
> One more thing, Philipp: When checking this, I came accross [1], where
> REMOVE_UNSUBSCRIBE is used, which is not defined anywhere - should that be
> REMOVE_NO_DELETE instead? You introduced that with bug 351499.

Yes, this should be REMOVE_NO_DELETE. Probably makes sense to add it to this patch.

r/a=me with comments considered. If you upload a new patch and/or push it, I'll do a new release rsn.

::: calendar/resources/content/calendarCreation.js
@@ +174,5 @@
> = cal_name;
>      gCalendar.setProperty('color', cal_color);
>      if (!gCalendar.getProperty("cache.always")) {
> +        gCalendar.setProperty("cache.enabled", gCalendar.getProperty("cache.supported") ?
> +                                               document.getElementById("cache").checked : false);

I believe you should check !== false here. If cache.supported is not explicitly set, it should default to true.

Maybe it would make sense to also (or instead) do something like this in the calendar manager: when cache.supported is false, don't wrap the calendar in the cache wrapper, even if cache.enabled is true.

This would guard us if there are other places where calendars are created, e.g. in extensions.
Attachment #8632426 - Flags: review?(philipp)
Attachment #8632426 - Flags: review+
Attachment #8632426 - Flags: approval-calendar-release+
Attachment #8632426 - Flags: approval-calendar-beta+
Attachment #8632426 - Flags: approval-calendar-aurora+
Updated patch with comments considered, but I haven't added the check to CalendarManager for now. The patch will take care to prevent data loss and remove inappropriate cache enabling anyway.
Attachment #8634374 - Flags: review+
Keywords: checkin-needed
Whiteboard: [checkin-needed:comm-central][checkin-needed:comm-aurora][checkin-needed:comm-beta][checkin-needed:comm-esr38]
Pushed to comm-central changeset b88d84e8d337
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 4.4
Note this bug is NOT part of 4.1, since we never really had a beta release for it.
...and will include manually in
Target Milestone: 4.0.2 →
Whiteboard: [checkin-needed:comm-central][checkin-needed:comm-aurora][checkin-needed:comm-beta][checkin-needed:comm-esr38]
With Lightning (from FTP) installed in TB 38.1.0, there's something strange: When I check "Offline Support" for my Baikal calendar (, the box is unchecked when I open calendar properties once again. This doesn't happen with Lightning 4.0.1. I can't do more testing now because I really have to leave. Could you check once again if the patch for this bug works as expected?
See bug 1187072.
No longer blocks: ltn402
You need to log in before you can comment on or make changes to this bug.