4668 - onload handler firing too early

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[Chris Waterson]

There are a bunch of timing issues (including onload handling and broadcaster
hookup) that happen if you lazily instantiate content nodes during layout.

Comment 1

[David Hyatt]

Setting this to have a P2 priority and setting target milestone of M5.
Mail/news is hosed without this working correctly.

Comment 2

[David Hyatt]

I don't think this has anything to do with RDF/XUL at all.  I went over into the
nsWebShellWindow code and was surprised to see that OnConnectionsComplete has
been commented out.  I tried to find out where the heck the onload handler code
went to, and I discovered that someone had moved it into EndDocumentLoad.

Moreover, the startup code is being called the first time the webshell receives
an end document load notification.  This appears to have been done by Radha.

What the heck happened to OnConnectionsComplete, and why has it been
invalidated?  Why are we executing the startup code on an end document load?
The end of a document load is not the same thing as the closure of all open
connections (which tells you only once when all of the subdocuments, images,
applets, etc.) have loaded.

The onload handler for a document is only supposed to fire once every last
subdocument, applet, image, frameset, iframe, and plugin has finished loading.
It should not be called the moment an end document load is received for the
parent document.

Maybe I'm misunderstanding the EndDocumentLoad notification in nsWebShellWindow.
If so, please enlighten me.

Comment 3

[scottputterman]

I just wanted to add another case where I'm seeing this happen.  I've
implemented the Navigator menu item in the Messenger Communicator Window.
Yesterday everything worked fine.  This morning it's dying because in
navigator.xul's Startup() which is called from onLoad,
appCore.setContentWindow(window.frames[0].frames[1]); sets a NULL ptr since the
frames haven't been created.  It's strange because running the browser by itself
works fine and it worked from mail yesterday.  The fact that it's working on
the browser is probably a matter of luck.

Comment 4

[scottputterman]

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

Comment 5

[scottputterman]

We actually have a bigger problem being caused by this bug.  Bug 4720 is a
duplicate of this bug and because of it we can't always type in the compose
window.  You can look at that bug, but the relevant text is:

"jefft and I just looked at this. The manifestations are:
1. First compose window works fine; the editor gets intantiated,
   everything works.
2. Second or subsequent compose windows don't work; they use the same editor
   appcores that was instantiated the first time, but in the call to
   setContentWindow, the content viewer in the webshell obtained from that
   window is null, so DoEditorMode fails.

I'm pretty sure this is not an editor problem. It might be either a side-effect
of the massive webshell leakage (bug 4127), or some other appcores/XUL problem
with the mail compose window. "

It turns out that SetContentWindow is being called in a function called from
onload.

Comment 6

[scottputterman]

The other this is that we had the compose window bug as an M4 stopper.  Is there
any chance we can get this done?

Comment 7

[Scott MacGregor]

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

Comment 8

[Radha on family leave (not reading bugmail)]

The DocLoaderObserver APIs have changed. There is nothing equivalent to
OnConnectionsComplete now. I'm working on this. I'll soon send a diff to
waterson and Putterman. Please let me know if this fixes your problems.

Thanks,

Comment 9

[lchiang]

Per putterman's question earlier, will this get fixed for M4?  It is causing a
bigger problem with Mail because we can't type in the editor.

Comment 10

[scottputterman]

I've tested out radha's fix (I don't think it's been checked in yet) and on my
machine I can now type in the compose window.  I wasn't able to before the fix.

Comment 11

[lchiang]

Will this fix be in the m4 branch?

Comment 12

[Radha on family leave (not reading bugmail)]

I'll check this in the tip as well as the M4 branch once waterson also gives me
a OK. I assume a OK from scott based on his comments here.

Comment 13

[scottputterman]

the fix looks good to me.

Comment 14

[Radha on family leave (not reading bugmail)]

Fix checked in. Please verify.

Comment 15

[lchiang]

Mail QA will verify the part of this bug as it relates to our duplicate bug
4720.  We need to wait for the next M4 builds with this fix before we can
verify.

Comment 16

[sujay]

verified using 4/13 builds for apprunner with editor mode.

I now do get subsequent instances of editor that come up that do
work this time.. However, I'm noticing that I cannot apply styles
to text for the second or subsequent editor window.

Lisa, I'll let your team mark this VERIFIED-FIXED after they
try it out for Mail Compose.

Comment 17

[lchiang]

We will verify with m4 builds available later on today since radha says the fix
is in M4 builds also.  The 4/13 builds are M5 builds.

Comment 18

[pmock]

Verified with the 1999-04-14-12-M4 build on Win32 NT 4.0 SP4 and Win98.
I verified that I can type in the message body and successfully send messages. I
also verified that the message received is correct.

Ninoschka verified this on the M4 Linux build.

Comment 19

[leger]

BULK MOVE: Changing component from XUL to XP Toolkit/Widgets: XUL.  XUL 
component will be deleted.