--Boundary-00=_aiXYBWWAA2k/uFM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline 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 <== > > > > > > > > > > > > > > > > The problem is HTMLFrameElementImpl::parseAttribute (for ATTR_ONLOAD). > > The code says: > > static_cast( getDocument() )->body()-> > > setHTMLEventListener(EventImpl::LOAD_EVENT, > > > > getDocument()->createHTMLEventListener(attr->value().string(),"onload")); > > And this obviously finds the main 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 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 . Anything I missed? The code path for isn't changed at all so it should still work :) -- David Faure, faure@kde.org, sponsored by Trolltech to work on KDE, Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org). --Boundary-00=_aiXYBWWAA2k/uFM Content-Type: text/x-diff; charset="iso-8859-1"; name="diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="diff" ? diff Index: html/html_baseimpl.cpp =================================================================== RCS file: /home/kde/kdelibs/khtml/html/html_baseimpl.cpp,v retrieving revision 1.198 diff -u -p -r1.198 html_baseimpl.cpp --- html/html_baseimpl.cpp 16 Apr 2004 22:48:25 -0000 1.198 +++ html/html_baseimpl.cpp 4 Oct 2004 16:13:06 -0000 @@ -279,17 +279,6 @@ void HTMLFrameElementImpl::parseAttribut scrolling = QScrollView::AlwaysOff; // when attached, has no effect break; - case ATTR_ONLOAD: - static_cast( getDocument() )->body() - ->setHTMLEventListener(EventImpl::LOAD_EVENT, - getDocument()->createHTMLEventListener(attr->value().string(),"onload")); - break; - case ATTR_ONUNLOAD: - static_cast( getDocument() )->body() - ->setHTMLEventListener(EventImpl::UNLOAD_EVENT, - getDocument()->createHTMLEventListener(attr->value().string(),"onunload")); - break; - default: HTMLElementImpl::parseAttribute(attr); } @@ -347,6 +336,25 @@ void HTMLFrameElementImpl::attach() } } +void HTMLFrameElementImpl::docCreated() +{ + DOMString onLoad = getAttribute(ATTR_ONLOAD); + if ( !onLoad.isNull() ) { + DocumentImpl* doc = contentDocument(); + static_cast( doc )-> + setHTMLEventListener(EventImpl::LOAD_EVENT, + doc->createHTMLEventListener(onLoad.string(),"onload")); + } + + DOMString onUnload = getAttribute(ATTR_ONUNLOAD); + if ( !onUnload.isNull() ) { + DocumentImpl* doc = contentDocument(); + static_cast( doc )-> + setHTMLEventListener(EventImpl::UNLOAD_EVENT, + doc->createHTMLEventListener(onUnload.string(),"onunload")); + } +} + void HTMLFrameElementImpl::setLocation( const DOMString& str ) { if ( url == str ) @@ -398,12 +406,11 @@ void HTMLFrameElementImpl::setFocus(bool DocumentImpl* HTMLFrameElementImpl::contentDocument() const { if ( !m_render ) return 0; - RenderPart* render = static_cast( m_render ); - if(render->widget() && ::qt_cast( render->widget()) ) + if ( render->widget() && ::qt_cast( render->widget()) ) { return static_cast( render->widget() )->part()->xmlDocImpl(); - + } return 0; } Index: html/html_baseimpl.h =================================================================== RCS file: /home/kde/kdelibs/khtml/html/html_baseimpl.h,v retrieving revision 1.86 diff -u -p -r1.86 html_baseimpl.h --- html/html_baseimpl.h 16 Apr 2004 22:53:48 -0000 1.86 +++ html/html_baseimpl.h 4 Oct 2004 16:13:06 -0000 @@ -94,6 +94,9 @@ public: virtual bool isSelectable() const; virtual void setFocus(bool); + /// Called by RenderFrame/RenderPartObject + void docCreated(); + DocumentImpl* contentDocument() const; DOMString url; Index: html/html_documentimpl.cpp =================================================================== RCS file: /home/kde/kdelibs/khtml/html/html_documentimpl.cpp,v retrieving revision 1.162 diff -u -p -r1.162 html_documentimpl.cpp --- html/html_documentimpl.cpp 9 Sep 2004 09:08:18 -0000 1.162 +++ html/html_documentimpl.cpp 4 Oct 2004 16:13:06 -0000 @@ -67,7 +67,6 @@ HTMLDocumentImpl::HTMLDocumentImpl(DOMIm : DocumentImpl(_implementation, v) { // kdDebug( 6090 ) << "HTMLDocumentImpl constructor this = " << this << endl; - bodyElement = 0; htmlElement = 0; m_doAutoFill = false; @@ -310,14 +309,8 @@ void HTMLDocumentImpl::close() // the first(IE)/last(Moz/Konq) registered onload on a and the // first(IE)/last(Moz/Konq) registered onload on a . - // The body has the listener for - b->dispatchWindowEvent(EventImpl::LOAD_EVENT, false, false); - - b = body(); // the onload code could have changed it (e.g. document.open/write/close) - - // The document has the listener for - if (b && b->id() == ID_FRAMESET) - getDocument()->dispatchWindowEvent(EventImpl::LOAD_EVENT, false, false); + //kdDebug() << "dispatching LOAD_EVENT on document " << getDocument() << " " << (view()?view()->part()->name():0) << endl; + getDocument()->dispatchWindowEvent(EventImpl::LOAD_EVENT, false, false); // don't update rendering if we're going to redirect anyway if ( view() && ( view()->part()->d->m_redirectURL.isNull() || Index: html/html_documentimpl.h =================================================================== RCS file: /home/kde/kdelibs/khtml/html/html_documentimpl.h,v retrieving revision 1.73 diff -u -p -r1.73 html_documentimpl.h --- html/html_documentimpl.h 9 Sep 2004 09:08:18 -0000 1.73 +++ html/html_documentimpl.h 4 Oct 2004 16:13:06 -0000 @@ -79,7 +79,6 @@ public: HTMLCollectionImpl::CollectionInfo *collectionInfo(int type) { return m_collection_info+type; } protected: - HTMLElementImpl *bodyElement; HTMLElementImpl *htmlElement; friend class HTMLMapElementImpl; friend class HTMLImageElementImpl; Index: rendering/render_frames.cpp =================================================================== RCS file: /home/kde/kdelibs/khtml/rendering/render_frames.cpp,v retrieving revision 1.186 diff -u -p -r1.186 render_frames.cpp --- rendering/render_frames.cpp 1 Oct 2004 22:03:51 -0000 1.186 +++ rendering/render_frames.cpp 4 Oct 2004 16:13:08 -0000 @@ -519,8 +519,11 @@ void RenderPart::setWidget( QWidget *wid #endif setQWidget( widget ); widget->setFocusPolicy(QWidget::WheelFocus); - if(widget->inherits("KHTMLView")) + if(widget->inherits("KHTMLView")) { connect( widget, SIGNAL( cleared() ), this, SLOT( slotViewCleared() ) ); + connect( static_cast( widget )->part(), + SIGNAL( docCreated() ), this, SLOT( slotDocCreated() ) ); + } setLayouted( false ); setMinMaxKnown( false ); @@ -549,6 +552,10 @@ void RenderPart::slotViewCleared() { } +void RenderPart::slotDocCreated() +{ +} + /***************************************************************************************/ RenderFrame::RenderFrame( DOM::HTMLFrameElementImpl *frame ) @@ -557,6 +564,11 @@ RenderFrame::RenderFrame( DOM::HTMLFrame setInline( false ); } +void RenderFrame::slotDocCreated() +{ + element()->docCreated(); +} + void RenderFrame::slotViewCleared() { if(m_widget->inherits("QScrollView")) { @@ -840,6 +852,14 @@ void RenderPartObject::layout( ) setLayouted(); } +void khtml::RenderPartObject::slotDocCreated() +{ + if ( element()->id() == ID_IFRAME) { + HTMLIFrameElementImpl *iframe = static_cast(element()); + iframe->docCreated(); + } +} + void RenderPartObject::slotViewCleared() { if(m_widget->inherits("QScrollView") ) { Index: rendering/render_frames.h =================================================================== RCS file: /home/kde/kdelibs/khtml/rendering/render_frames.h,v retrieving revision 1.58 diff -u -p -r1.58 render_frames.h --- rendering/render_frames.h 9 Aug 2004 08:39:46 -0000 1.58 +++ rendering/render_frames.h 4 Oct 2004 16:13:08 -0000 @@ -120,6 +120,7 @@ public: public slots: virtual void slotViewCleared(); + virtual void slotDocCreated(); }; class RenderFrame : public khtml::RenderPart @@ -134,9 +135,11 @@ public: { return static_cast(RenderObject::element()); } public slots: - void slotViewCleared(); + virtual void slotViewCleared(); + virtual void slotDocCreated(); }; +// Used for HTMLIFrameElementImpl (