Possible dataloss after editing calendar properties

RESOLVED FIXED in 4.0.1.2

Status

Calendar
General
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Robert Brand, Assigned: MakeMyDay)

Tracking

({dataloss, regression})

Lightning 4.0.0.1
4.0.1.2
dataloss, regression

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
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

2 years ago
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: 1180416

Comment 3

2 years ago
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.
(Assignee)

Comment 5

2 years ago
Created attachment 8632426 [details] [diff] [review]
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.
Assignee: nobody → makemyday
Status: NEW → ASSIGNED
Attachment #8632426 - Flags: review?(philipp)
(Assignee)

Updated

2 years ago
Severity: normal → critical
Keywords: dataloss, regression
(Assignee)

Comment 6

2 years ago
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 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.
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+
(Assignee)

Comment 8

2 years ago
Created attachment 8634374 [details] [diff] [review]
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.
Attachment #8634374 - Flags: review+
(Assignee)

Updated

2 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 4.4
Backported to releases/comm-aurora changeset 89a926b8c66f
Target Milestone: 4.4 → 4.3
Backported to releases/comm-beta changeset 3144fac037fa
Target Milestone: 4.3 → 4.2
Note this bug is NOT part of 4.1, since we never really had a beta release for it.
Backported to releases/comm-esr38 changeset f8f6f4767a76
Target Milestone: 4.2 → 4.0.2
...and will include manually in 4.0.1.1
Target Milestone: 4.0.2 → 4.0.1.1
Whiteboard: [checkin-needed:comm-central][checkin-needed:comm-aurora][checkin-needed:comm-beta][checkin-needed:comm-esr38]
(Reporter)

Comment 15

2 years ago
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?
(Assignee)

Comment 16

2 years ago
See bug 1187072.
(Assignee)

Updated

2 years ago
No longer blocks: 1180416
You need to log in before you can comment on or make changes to this bug.