an exception occurs when closing the alarm window containing alarms for several occurences of the same event

RESOLVED FIXED in 1.0

Status

Calendar
Alarms
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: Wolfgang Sourdeau, Assigned: mmecca)

Tracking

unspecified
Bug Flags:
blocking-calendar1.0 +

Details

(Whiteboard: [needed beta][no l10n impact])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; fr; rv:1.9.0.10) Gecko/2009042805 Iceweasel/3.0.9 (Debian-3.0.9-1)
Build Identifier: 

The exception triggers the display of a window with the error "MODIFICATION_FAILED", although no description is actually displayed.
The related exception in the error console is as follows:

Erreur : Une erreur est survenue lors de l'écriture sur l'agenda Test Alarms ! Code d'erreur : MODIFICATION_FAILED. Description : 
Fichier source : file:///home/wsourdeau/.thunderbird/g1ny9uwx.default/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calItemModule.js -> file:///home/wsourdeau/.thunderbird/g1ny9uwx.default/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/js/calCalendarManager.js
Ligne : 969

This exception will be displayed as many times as there are occurences involved - 1.

Reproducible: Always
Component: General → Alarms
QA Contact: general → alarms
Wolfgang, does this still happen with 1.0b1? Does it happen with each and every alarm? On what type of calendar? If this only happens for certain recurring events, please attach a testcase.
(Reporter)

Comment 2

8 years ago
(In reply to comment #1)
> Wolfgang, does this still happen with 1.0b1? Does it happen with each and every
> alarm? On what type of calendar? If this only happens for certain recurring
> events, please attach a testcase.
Let me reply later.
(Assignee)

Comment 3

7 years ago
Confirming - I've been tripping over this working on Bug 554267. 

The snooze all function is currently handled by the alarm dialog, and relies on widgets being removed live as the items are modified while looping through alarms to snooze. With the window closed this doesn't happen, and all of the occurrences of a repeating item try to modify the same parent item. The first one succeeds, and the rest through a modification failed error.

Using the Snooze All button will still work if this only involves a single repeating event, but that will break also with multiple repeating events with multiple occurrences, with a similar root problem.

I have a work in progress patch for Bug 554267 I'll be posting shortly that moves batch alarm processing out of the dialog and into calAlarmService that should help this .
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

7 years ago
Assignee: nobody → matthew.mecca
Matthew, what do you think about changing the UI here? Instead of showing an alarm row for each occurrence of the item, maybe we could reduce this to one row per item and show more information on the row itself (i.e how many occurrences are being signaled, etc).

This would also relieve bug 496893 and clean up UI clutter a bit.
(Assignee)

Comment 5

7 years ago
This makes sense for events, but we should be careful grouping alarms for tasks. One use case might be setting one reminder at the start time of a repeating task, and another some amount of time after the due date as a warning that the task is overdue - I'm not sure we'd want those alarms grouped on the same line.
duping to the bug with more people and more dups to.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 581943
(Assignee)

Comment 7

6 years ago
Un-marking as dup per bug 581943 comment #20
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Assignee)

Comment 8

6 years ago
Created attachment 568555 [details] [diff] [review]
Fix v1

This is a simpler and less risky fix than the batch alarm handling fix in bug 554267. 

Due to the way we handle alarm acknowledgement and snooze properties on the parent item, we only need to dismiss or snooze the first occurrence if there are multiple unacknowledged occurrences in the dialog. 

One difference between this and the batch alarm fix is that if we snooze several occurrences of the same item, only one will fire an alarm after the snooze period, the other occurrences are effectively dismissed (this may actually be the better way to do it - see related discussion on Bug 397030).
Attachment #568555 - Flags: review?(philipp)
(Assignee)

Updated

6 years ago
Flags: blocking-calendar1.0?
OS: Linux → All
Hardware: x86 → All
(Assignee)

Updated

6 years ago
Blocks: 581943
Flags: blocking-calendar1.0? → blocking-calendar1.0+
Whiteboard: [needed beta][no l10n impact]
Comment on attachment 568555 [details] [diff] [review]
Fix v1


>+        if (node.parentNode && node.item && node.alarm &&
>+            !parentItems[node.item.parentItem.hashId]) {
!(node.item.parentItem.hashId in parentItems) might be safer to avoid strict warnings


Otherwise r=philipp and approval for all branches, I like this solution!
Attachment #568555 - Flags: review?(philipp) → review+
(Assignee)

Comment 10

6 years ago
Created attachment 568981 [details] [diff] [review]
Fix v2

Addresses review comment.
Attachment #568555 - Attachment is obsolete: true
Attachment #568981 - Flags: review+
(Assignee)

Comment 13

6 years ago
Pushed to comm-beta - http://hg.mozilla.org/releases/comm-beta/rev/365a0c588212
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b8
You need to log in before you can comment on or make changes to this bug.