Closed Bug 496897 Opened 15 years ago Closed 13 years ago

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

Categories

(Calendar :: Alarms, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: WSourdeau, Assigned: mmecca)

References

Details

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

Attachments

(1 file, 1 obsolete file)

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.
(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.
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: 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.
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
Closed: 13 years ago
Resolution: --- → DUPLICATE
Un-marking as dup per bug 581943 comment #20
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Attached patch Fix v1 (obsolete) — Splinter Review
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)
Flags: blocking-calendar1.0?
OS: Linux → All
Hardware: x86 → All
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+
Attached patch Fix v2Splinter Review
Addresses review comment.
Attachment #568555 - Attachment is obsolete: true
Attachment #568981 - Flags: review+
Pushed to comm-beta - http://hg.mozilla.org/releases/comm-beta/rev/365a0c588212
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b8
You need to log in before you can comment on or make changes to this bug.