449574 - aDOMWindow errors

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.17pre) Gecko/20080806 Calendar/0.9pre

Error: aDOMWindow.arguments has no properties
Source File: file:///F:/Software/Buero/Kalender/Sunbird/Branch/components/calItemModule.js -> file:///F:/Software/Buero/Kalender/Sunbird/Branch/js/calCalendarManager.js
Line: 848

Error: aDOMWindow.arguments[0].QueryInterface is not a function
Source File: file:///F:/Software/Buero/Kalender/Sunbird/Branch/components/calItemModule.js -> file:///F:/Software/Buero/Kalender/Sunbird/Branch/js/calCalendarManager.js
Line: 850

STR: ?

Comment 1

[Stefan Sitter [:ssitter]]

Issue was seen before in Bug 442881 that was resolved as WFM.

Comment 2

[Bas van den Bosch]

This error occurs when opening the calendar-properties dialog, also with lightning (2008090118). This is not the same dialog error as bug 442881.

Comment 3

[Martin Schröder [:mschroeder]]

I also observe those errors regularly but couldn't find a way to reproduce them reliably.

Comment 4

[Stefan Sitter [:ssitter]]

I have seen them randomly after observing other errors before.

Comment 5

[gekacheka]

DIAGNOSIS

Bug seems to be that calMgrCalendarObserver.onCloseWindow is prepared
to get called when only one type of dialog is closed, but is getting
called on many types of dialog.  window.arguments[0] is different for
every dialog.
http://mxr.mozilla.org/mozilla/source/calendar/base/src/calCalendarManager.js#841
calMgrCalendarObserver is added as a listener on nsIWindowMediator in
calMgrCalendarObserver.announceError.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/calendar/base/src/calCalendarManager.js&rev=1.67&mark=981#981

nsIWindowMediator keeps track of open XUL windows, and calls
onCloseWindow for *any* closed XUL windows.  It is also called before
a window is loaded (presumably to remove any old content).

calMgrCalendarObserver.announceError is called by onError for
calIObserver, so it is called when there are errors reading, parsing,
or writing the calendar.

So this error only occurs after there is a problem reading, parsing,
or writing a calendar.  After that, this error occurs whenever any
application xul window or dialog is closed (besides the error window
it is expecting).

STEPS TO REPRODUCE

0. subscribe to an unwritable file
  0.1 Using text editor, create an empty file called "unwritable.ics".
  0.2 Using file system, make it read-only.
  0.3 In Sunbird, File / Open Calendar File
  0.4 Navigate to "unwritable.ics" and open it.
1. If sunbird marks it read-only (when it restarts), mark it writable.
  1.1 popup menu on calendar "unwritable"
  1.2 uncheck [ ] read-only.
2. select the calendar "unwritable" (dark selection background)
3. click new event.  
  expect: Should popup dialog with calendar set to "unwritable"
4. click save and close.
  expect: "An error has occurred" dialog pops up with message
       "An error occurred when writing to the calendar unwritable!"
  expect: two messages appear in Error console:
    - "There has been an error reading data..."
    - "An error occurred when writing to the calendar unwritable!
       Error code MODIFICATION FAILED.  ..."
  bug symptom:  Error console shows:
    "aDOMWindow.arguments[0].QueryInterface is not a function"
5. close the "An error has occurred" dialog.
  expect: No error console message -- this is the dialog the listener
    was expecting.
6. Open and close any another dialog now produces a similar bug
   symptom as in step 4.  Looks like the listener is still listening.
6.1 bug symptom: Help / About Sunbird produces in Error Console the message
      "aDOMWindow.arguments has no properties"
6.2 bug symptom: Many dialogs (Go / Date, New Event, etc.)
    produce the Error Console message:
      "aDOMWindow.arguments[0].QueryInterface is not a function"

REDESIGN

What the code is trying to do is pop up a calendar-error-prompt dialog
for each error, but avoid popping up the same message in more than one
dialog at a time.  When the user closes a dialog, the listener removes
its message from the set of "announcedMessages", so if the message
comes up later, it will be shown in a new dialog.

Problems with the current design include:
1. The listener runs on all closing events,
   not those just from calendar-error-prompt windows.
2. The listener is only removed when announcedMessages is empty.
   Messages are added to announcedMessages when they are shown on the
   Error console, but only some messages are shown in a
   calendar-error-prompt dialog.  Yet the message is only removed
   after being shown in the dialog, so if there are any non-dialog
   messages (the usual case), the listener is never removed.
3. Messages previously logged are not shown in error console.  
   This may interfere with bug reports and debugging by users, 
   who may be unable to reproduce console error messages consistently
   by repeating the same actions.

   (An issue with deduplication of console messages at the observer is
   that it is impossible for the observer to know if two messages
   originated from the same user action [two errors from same file] or
   two different actions [two errors trying to load file on two
   occasions].  Time gaps are not a reliable heuristic since network
   delays can cause time gaps as large as the time between user
   retries.)

Solutions:
1. Rather than try to detect whether the window closing event came from
   the right kind of window, it is simpler to add listeners to the prompt
   windows instead of the windowMediator so they will only receive events
   when a calendar-error-prompt window is closed.
2. Only add messages which are displayed in a prompt window to
   announcedMessages, and delete them when the prompt is closed.
3. Log all messages.  (Serious messages can be errors, others warnings.)

Comment 6

[gekacheka]

Attachment: patch v1: use window unload listeners, suppress only prompt-windows

(patch -l -p 1 -i file.patch)

Patch implements redesign described above.

With this patch a listener waits for a DOM "unload" event rather than
nsIWindowMediator onCloseWindow events, so a listener is attached to
the prompt window, not the mediator, and each listener only receives
events from its calendar-error-prompt window, not all window closing
events.  Each listener also captures the prompt parameters so it
doesn't have to be recovered from the window arguments.

One complication is that the unload/close event is also fired before
the calendar-error-prompt window is first opened, presumably to
cleanup a reused window.  (The mediator listener also received a
pre-load closing event as seen in step 4 above.)  So this patch first
adds a "load" event listener to the window; once it is loaded, it then
adds an "unload" event listener.  At that time the next "unload" event
means the message is no longer visible (the user closed the
calendar-error-prompt window).

A message is added to announcedMessages only if it is displayed in
a prompt window.  As before, it avoids showing two prompt windows for
the same message.

All messages are displayed in the console.  Serious messages that
might cause a prompt are displayed as errors, others as warnings.

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 337242 [details] [diff] [review]
patch v1:  use window unload listeners, suppress only prompt-windows

Berend is more into the error prompts, delegating review to him. At a glance it looks good though.

Comment 8

[Berend Cornelius]

Comment on attachment 337242 [details] [diff] [review]
patch v1:  use window unload listeners, suppress only prompt-windows

Patch looks good and works fine. I will check it in.

Comment 9

[Martin Schröder [:mschroeder]]

Berend has already checked the patch in. -> RESOLVED FIXED