353051 - 'Dismiss All' doesn't dismiss all alarms (skips every second)

Status: VERIFIED FIXED

(Calendar :: Alarms, defect)

Description

[Stefan Sitter [:ssitter]]

'Dismiss All' doesn't dismiss all alarms (skips every second)

Steps to Reproduce:
1. Create four events with alarms set (e.g. event '1', '2', '3' and '4')
2. Wait until all four alarms are fired and press 'Dismiss All'
3. Restart Sunbird

Actual Results:
Alarm popup for event '2' and '4' are displayed.

Expected Results:
No alarm popup because all alarms were dismissed.

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060917 Calendar/0.3a2+

Console output: During alarm creation:

    considering alarm for item:1
     offset:PT15M, which makes alarm time:2006/09/11 11:30:00 UTC
    now is 2006/09/17 16:33:40 UTC
    Last ack was:null
    alarm is in the past, and unack'd, firing now!
    snooze time is:null
    considering alarm for item:2
     offset:PT15M, which makes alarm time:2006/09/12 11:30:00 UTC
    now is 2006/09/17 16:34:00 UTC
    Last ack was:null
    alarm is in the past, and unack'd, firing now!
    snooze time is:null
    considering alarm for item:3
     offset:PT15M, which makes alarm time:2006/09/13 11:45:00 UTC
    now is 2006/09/17 16:34:16 UTC
    Last ack was:null
    alarm is in the past, and unack'd, firing now!
    snooze time is:null
    considering alarm for item:4
     offset:PT15M, which makes alarm time:2006/09/14 11:45:00 UTC
    now is 2006/09/17 16:34:20 UTC
    Last ack was:null
    alarm is in the past, and unack'd, firing now!

After pressing Dismiss All:

    snooze time is:null
    considering alarm for item:1
     offset:PT15M, which makes alarm time:2006/09/11 11:30:00 UTC
    now is 2006/09/17 16:35:08 UTC
    Last ack was:2006/09/17 16:35:08 UTC
    1 - alarm previously ackd2
    snooze time is:null
    considering alarm for item:3
     offset:PT15M, which makes alarm time:2006/09/13 11:45:00 UTC
    now is 2006/09/17 16:35:08 UTC
    Last ack was:2006/09/17 16:35:08 UTC
    3 - alarm previously ackd2

Comment 1

[Stefan Sitter [:ssitter]]

Regression range:

WORKS in Sunbird/0.3a2+ (2006-08-31-06)
FAILS in Sunbird/0.3a2+ (2006-08-31-21)

Comment 2

[Stefan Sitter [:ssitter]]

This was regressed by the checkin for Bug 298358 to
mozilla/calendar/base/content/calendar-alarm-dialog.js

After that patch onDismissAll() calls onDismissWidget() instead of dismissing each alarm itself. But onDismissWidget() will also remove the widget from dialog. Therefore the for-each-loop in onDismissAll() works only on every second item.

Comment 3

[Stefan Sitter [:ssitter]]

Attachment: loop in reverse order

Comment 4

[Matthew (lilmatt) Willis]

Comment on attachment 238906 [details] [diff] [review]
loop in reverse order

>-  for each (kid in box.childNodes) {
>+  for (var i = box.childNodes.length-1; i >= 0; i--) {
>+    var kid = box.childNodes[i];
I _think_ you need to declare var kid outside the for loop, or you'll run into javascript strict warnings.

Otherwise, nice fix. r=lilmatt with var above for.

Comment 5

[Joey Minta]

Comment on attachment 238906 [details] [diff] [review]
loop in reverse order

-  for each (kid in box.childNodes) {
+  for (var i = box.childNodes.length-1; i >= 0; i--) {
+    var kid = box.childNodes[i];
     if (!kid || !kid.item) {
         continue;
     }
If you're explicitly looping through childNodes here, the kid/kid.item checks ought to be superfluous.  They were a result of using forEach.  Yes?

Comment 6

[Stefan Sitter [:ssitter]]

Attachment: loop in reverse order, v2

Include changes to address Comment #4 and Comment #5. 
The |kid| check is not necessary with this loop. This also means there is no need for |kid| at all. Also removes obsolete code in that function.

Comment 7

[Joey Minta]

Comment on attachment 239049 [details] [diff] [review]
loop in reverse order, v2

Yay for code removal. r2=jminta

Comment 8

[Matthew (lilmatt) Willis]

Comment on attachment 239049 [details] [diff] [review]
loop in reverse order, v2

r1=lilmatt

Comment 9

[Matthew (lilmatt) Willis]

Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED

Comment 10

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060929 Sunbird/0.3

Comment 11

[Damian Szczepanik]

Litmus testcase 2695 created