1182264 - Possible dataloss after editing calendar properties

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Robert Brand]

GerryB from thunderbird-mail.de 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.

Comment 1

[Decathlon]

It seems caused by the patch for bug 768207

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Aw man, I thought that was such a simple patch. Can you create a patch for this?

Comment 3

[Decathlon]

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.

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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.

Comment 5

[[:MakeMyDay]]

Attachment: FixAccidentallyEnabledCachingIfCacheIsUnsupported-V1.diff

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 4.0.1.1 immediately to avoid bad user expirience esp. for the new users and bad publicity accordingly.

Comment 6

[[:MakeMyDay]]

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.

[1] http://mxr.mozilla.org/comm-central/source/calendar/base/src/calCalendarManager.js#851

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8632426 [details] [diff] [review]
FixAccidentallyEnabledCachingIfCacheIsUnsupported-V1.diff

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 @@
>      gCalendar.name = 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.

Comment 8

[[:MakeMyDay]]

Attachment: FixAccidentallyEnabledCachingIfCacheIsUnsupported-V2.diff

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.

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Pushed to comm-central changeset b88d84e8d337

Comment 10

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Backported to releases/comm-aurora changeset 89a926b8c66f

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Backported to releases/comm-beta changeset 3144fac037fa

Comment 12

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Note this bug is NOT part of 4.1, since we never really had a beta release for it.

Comment 13

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Backported to releases/comm-esr38 changeset f8f6f4767a76

Comment 14

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

...and will include manually in 4.0.1.1

Comment 15

[Robert Brand]

With Lightning 4.0.1.2 (from FTP) installed in TB 38.1.0, there's something strange: When I check "Offline Support" for my Baikal calendar (https://www.nadelundhirn.de/....), 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?

Comment 16

[[:MakeMyDay]]

See bug 1187072.