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

Status: RESOLVED FIXED

(Calendar :: Dialogs, defect)

Description

[Thomas]

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

[[:MakeMyDay]]

Attachment: FailToSaveOrCloseEventDialogWhenSavedOnce-V1.diff

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.

Comment 2

[Decathlon]

Comment on attachment 8370720 [details] [diff] [review]
FailToSaveOrCloseEventDialogWhenSavedOnce-V1.diff

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.

r+

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).

Comment 3

[[:MakeMyDay]]

Attachment: FailToSaveOrCloseEventDialogWhenSavedOnce-V2.diff

Updated patch with reflecting to code style comments

Comment 4

[[:MakeMyDay]]

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

Comment 5

[Decathlon]

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

pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/fa839fc34b66

Comment 6

[Decathlon]

Already pushed to comm-central.