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

RESOLVED FIXED in 3.2

Status

Calendar
Dialogs
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Thomas, Assigned: MakeMyDay)

Tracking

Lightning 1.9

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

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

Comment 1

4 years ago
Created attachment 8370720 [details] [diff] [review]
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.
Attachment #8370720 - Flags: review?(bv1578)

Comment 2

4 years ago
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).
Attachment #8370720 - Flags: review?(bv1578) → review+
(Assignee)

Comment 3

4 years ago
Created attachment 8372800 [details] [diff] [review]
FailToSaveOrCloseEventDialogWhenSavedOnce-V2.diff

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

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:
http://hg.mozilla.org/comm-central/rev/fa839fc34b66
Assignee: nobody → makemyday
Status: UNCONFIRMED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.1
(Reporter)

Updated

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.