343246 - Quickly fired alarms don't always appear in the same window

Status: VERIFIED FIXED

(Calendar :: Internal Components, enhancement)

Description

[dror3go]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060629 Sunbird/0.3a2+

When opening Sunbird with more than one event missed, instead of one popup with a list of all the missed events (with checkboxes for removing alarm for next time), Sunbird opens many popup windows.

Reproducible: Always

Comment 1

[Joey Minta]

I've seen this before in testing.

Comment 2

[Stefan Sitter [:ssitter]]

*** Bug 341571 has been marked as a duplicate of this bug. ***

Comment 3

[Reed Loden [:reed]]

The bugspam monkeys have struck again. They are currently chewing on default assignees for Calendar. Be afraid for your sanity!

Comment 4

[Greg Trounson]

(In reply to comment #2)
> *** Bug 341571 has been marked as a duplicate of this bug. ***
> 

Confirmed in 2006-08-01/Win32 Nightly.

Open Sunbird and loaded a remote calendar with several recurring events.
25 Sunbird windows are present in Windows XP taskbar - 1 Sunbird proper, and 24 alarm windows.  None of which respond until program is killed, even after waiting 30 minutes.

Just had to kill and restart sunbird (without loading remote calendar) to obtain version string:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060801 Calendar/0.3a2+

Comment 5

[Greg Trounson]

Forgot to also state:

I get the "Unresponsive Script" dialogue for each event if I click "stop script" for each of them.  If I check the "Never show this dialog again" box and click "continue", that's when it freezes up indefinitely.

Comment 6

[Stefan Sitter [:ssitter]]

(In reply to comment #5)
That is Bug 346801 and not related to the issue in this bug. 

Don't resent, but please don't add useless 'the-problem-still-exists' comments like Comment #4 if the bug is already confirmed but not yet fixed. [https://bugzilla.mozilla.org/page.cgi?id=etiquette.html]

Comment 7

[Matthew (lilmatt) Willis]

Assigning to jminta at his request via irc

Comment 8

[Joey Minta]

Attachment: store opening window

This patch is pretty well described by the comment included in the code.  There's a small time-frame after we call openWindow() but before the window will be returned by getMostRecentWindow().  If an alarm is fired in this time-frame, it will end up in a second window, which is bad.  This patch lets us keep track of windows that are being opened.

Comment 9

[Matthew (lilmatt) Willis]

Comment on attachment 232652 [details] [diff] [review]
store opening window

+    // This is a work-around for the fact that there is a delay between when
+    // we call openWindow and when it appears via getMostRecentWindow.  If an
+    // alarm is fired in that window, it will actually end up in another window. 

Unless I'm misinterpreting the above comment, you probably want to reword "If an alarm is fired in that window" somehow, so as to not confuse "window of time" with "alarm windows". If I'm interpreting it wrong, then you _really_ need to reword things to make it clearer. :)


         else {
+            // The window isn't open yet
+            var alarmObserver = this;
             var addAlarm = function() {
-                gAlarmWindow.addAlarm( event );
+                openAlarmWindow.addAlarm( event );
+                // Now the window is open
+                alarmObserver._WindowOpening = null;

What purpose does the "alarmObserver" variable serve here?  Could the last line not be "this._WindowOpening = null;"?


The comments apply to both files :)

r=lilmatt with those nits

Comment 10

[Joey Minta]

Comment on attachment 232652 [details] [diff] [review]
store opening window

r=dmose

Comment 11

[Joey Minta]

Patch checked in.

Comment 12

[Damian Szczepanik]

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

Comment 13

[Matthew (lilmatt) Willis]

-> VERIFIED