Closed Bug 449567 Opened 16 years ago Closed 15 years ago

'dismiss all' alarms of recurring events -> error console output

Categories

(Calendar :: Alarms, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: andreas.treumann, Assigned: sunil)

Details

Attachments

(1 file, 2 obsolete files)

STEPS TO REPRODUCE:
===================

- go to lightning options/preferences
- open the alarm tabpage
- enable 'Default alarm setting for events'
- create an recurring event in the past and save it
- alarm dialog fires
- push 'dismiss all'

RESULT:
=======

- output in error console:

Error: alarmRichlist.childNodes[i] has no properties
Source File: chrome://calendar/content/calendar-alarm-dialog.js
Line: 84

EXPECTED RESULT:
================

- no error output

REPRODUCIBLE:
=============

- always

'Snooze all' also leads to an output:

Error: alarmRichlist.childNodes[i] has no properties
Source File: chrome://calendar/content/calendar-alarm-dialog.js
Line: 204
I think the reason is Bug 397030. You try to snooze/dismiss all entries in the dialog. But after snoozing/dismissing the first entry the other entries get removed too due to Bug 397030. Then it tries to snooze/dismiss the second entry that no longer exists.
Don't think those relate (this looks like a plain code bug). I don't even think bug 397030 is a bug at all.
Attached patch Patch to fix the problem (obsolete) — Splinter Review
The attached patch fixed the problem for me. Works for the two cases of snooze all and dismiss all. This should work whether or not bug 397030 is actually a bug.
Attachment #361499 - Flags: review?(mozilla)
Assignee: nobody → sunil
Status: NEW → ASSIGNED
I just found out that if there is an error in removing the item for some reason (I had this happening in caldav backend), this patch is leading to a catastrophic infinite loop.
Attachment #361499 - Flags: review?(mozilla) → review?(philipp)
Comment on attachment 361499 [details] [diff] [review]
Patch to fix the problem

moving review over to fallen
Try using

for each (let node in Array.slice(alarmRichlist.childNodes)) {
  if (node.item) {
      ...
  }
}

I'd hope that slicing the array creates a fixed copy so we only dismiss the items once.
Attachment #361499 - Flags: review?(philipp) → review-
Comment on attachment 361499 [details] [diff] [review]
Patch to fix the problem

r- since this doesn't correctly fix the issue. Please do follow up on this though :-)
Attached patch Patch to fix the problem v2 (obsolete) — Splinter Review
(In reply to comment #6)
> for each (let node in Array.slice(alarmRichlist.childNodes)) {
>   if (node.item) {
>       ...
>   }
> }

I think something like this can't be done because childNodes is not an array but a NodeList. So, I made an explicit copy in the attached patch.
Attachment #361499 - Attachment is obsolete: true
Attachment #361751 - Flags: review?(philipp)
This trick is quite new to me too, but Array.slice() actually transforms a NodeList into something array-like that can be iterated. Go ahead and give it a try :-)
Ahh! I can now imagine a SpiderMonkey monkey with a wide grin on his face. :)
Attachment #361751 - Attachment is obsolete: true
Attachment #362187 - Flags: review?(philipp)
Attachment #361751 - Flags: review?(philipp)
Attachment #362187 - Flags: review?(philipp) → review+
Comment on attachment 362187 [details] [diff] [review]
Patch to fix the problem v3

Looks good, r=philipp
Thanks for the patch!

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/e61d893526ba>

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Thanks Philipp.
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.