gmail problem investigated: frame onload

Mon Oct 4 17:21:42 BST 2004

On Monday 04 October 2004 16:58, Waldo Bastian wrote:
> On Monday 04 October 2004 16:41, David Faure wrote:
> > With Rainder Endres' help, I managed to track down the reason gmail won't
> > load. The onload event listener for frames and iframes is registered on the
> > wrong body: the main body instead of the frame's body. Which means only one
> > gets executed, and the JS code waits for ever.
> >
> > Testcase [gmail uses iframes, but it's the same with frames] :
> >
> > ==> blank.html <==
> > Hello world
> >
> > ==> frameset.html <==
> > <html>
> > <frameset rows="*,*,*">
> > <frame name="main" src="blank.html" onload="alert('loaded main')">
> > <frame name="blank" src="blank.html" onload="alert('loaded blank')">
> > </frameset>
> > </html>
> >
> > The problem is HTMLFrameElementImpl::parseAttribute (for ATTR_ONLOAD).
> > The code says:
> >         static_cast<HTMLDocumentImpl*>( getDocument() )->body()->
> >               setHTMLEventListener(EventImpl::LOAD_EVENT,
> >            
> > getDocument()->createHTMLEventListener(attr->value().string(),"onload"));
> > And this obviously finds the main <body> element, since getDocument() for
> > the frame element finds the main document.
> >
> > Did this ever work in KHTML? I thought it did, but looking at the code, I
> > fail to see how. Since the attributes are parsed before the frame is
> > "attached", so there is no KHTMLPart for the frame yet... i.e. no way to
> > find the right document (or body) from that code. So do we need to store
> > the value of the load and unload listeners until attach? We would then need
> > to have some code somewhere that sets the listener on the frame's body when
> > creating it..... I'm working now on a patch that attempts to do that.
> 
> See also the last (reverted) patch to html_baseimpl.cpp Same issue.
> http://bugs.kde.org/show_bug.cgi?id=72440 has the full discussion.

Thanks for the info. However all the patches in that BR have the same problem:
they set the listener on the wrong document or body. Another bug in the last
patch in that BR was that the close() change would make <body onload> get
triggered twice - this is why I completely remove one of the two dispatchWindowEvent
calls in my patch.

The attached patch fixes the bug correctly (IMHO), by delaying the creation of 
the event listener until the frame's document actually exists (!)

The patch also fixes the crash in #72440 since we don't use body() anymore at all.

I tested onload on frames, onload on iframes, onload on <body>. Anything I missed?
The code path for <frameset onload> isn't changed at all so it should still work :)

-- 
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: diff
Type: text/x-diff
Size: 8290 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20041004/f53549c1/attachment.diff>