Error on "Save on close" after "Save" when creating of editing event.



5 years ago
4 years ago


(Reporter: Thomas, Assigned: MakeMyDay)


Lightning 1.9



(1 attachment, 1 obsolete attachment)



5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.57 Safari/537.17

Steps to reproduce:

1) Create or edit an event
2) Save using CTRL+S
3) Save using Save and close or CTRL+ENTER (regardless of content changed)
4) Dialog won't close and won't save.

Actual results:

Since dialog won't close, I checked the error console:
Error occured:
NS_ERROR_OBJECT_IS_IMMUTABLE: 'Can not modify immutable data container' when calling method: [calIAlarm::related]
Source: chrome://calendar/content/calendar-dialog-utils.js
Row: 63

Expected results:

Changes made between 2) and 3) should be saved, they are not. 
Dialog should save and close, it does not.

Comment 1

4 years ago
Created attachment 8370720 [details] [diff] [review]

Confirming. Once the dialog is saved using the save command (either from file menu or short cuts), any subsequent attempt to save further changes agein will fail. The same applies if want to close the dialog (with exception while clicking on the windows x) after saving once.

This is caused within event calendar-dialog-utils in createReminderFromMenuitem. If the changes in the dialog were saved once, the passed reminder is immutable, so the first attempt to assign a value will stop any further processing.

The attched patch should take care.
Attachment #8370720 - Flags: review?(bv1578)

Comment 2

4 years ago
Comment on attachment 8370720 [details] [diff] [review]

As far as I can see the bug is reproducible, according to steps to reproduce, only with an event which has a single reminder already set when the dialog is being opened. It doesn't occur when setting for the first time a reminder on a new event or when there are multiple reminders.
In my case I don't see any error in the console with Lightning 3.1a1 and Thunderbird 29.0a1.

For the new events the bug doesn't occur because the function createReminderFromMenuitem() always creates the reminder with cal.createAlarm() (since aMenuitem.reminder is always undefined) which returns a mutable reminder.

Given the kind of error, I think the patch fixes in the right way.


Some minor nit about the code style:

>+    let isImmutable = (!reminder.isMutable) ? true : false;

Here you could avoid the NOT (and the parenthesis) by swapping "true" and "false"

>+    if(isImmutable) reminder = reminder.clone();
>+    if(isImmutable) reminder.makeImmutable;

Usually the guidance for the Calendar code is to use the brackets and a new line with indentation even for a single line of code for statements like "if", "for" etc. (also a space between "if" and the parenthesis).
Attachment #8370720 - Flags: review?(bv1578) → review+

Comment 3

4 years ago
Created attachment 8372800 [details] [diff] [review]

Updated patch with reflecting to code style comments
Attachment #8370720 - Attachment is obsolete: true

Comment 4

4 years ago
Can the bug reporter or anybody with the according permission please set the checkin-needed keyword? Thank you.

Comment 5

4 years ago
I've added the brackets to the "if" statements in the patch.

pushed to comm-central:
Assignee: nobody → makemyday
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.1


4 years ago
Keywords: checkin-needed

Comment 6

4 years ago
Already pushed to comm-central.
Keywords: checkin-needed
Target Milestone: 3.1 → 3.2
You need to log in before you can comment on or make changes to this bug.