[prev in list] [next in list] [prev in thread] [next in thread]
List: kfm-devel
Subject: Re: gmail problem investigated: frame onload
From: David Faure <faure () kde ! org>
Date: 2004-10-04 16:21:42
Message-ID: 200410041821.46075.faure () kde ! org
[Download RAW message or body]
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@kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
["diff" (text/x-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<HTMLDocumentImpl*>( getDocument() )->body()
- ->setHTMLEventListener(EventImpl::LOAD_EVENT,
- getDocument()->createHTMLEventListener(attr->value().string(),"onload"));
- break;
- case ATTR_ONUNLOAD:
- static_cast<HTMLDocumentImpl*>( 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<HTMLDocumentImpl*>( doc )->
+ setHTMLEventListener(EventImpl::LOAD_EVENT,
+ \
doc->createHTMLEventListener(onLoad.string(),"onload")); + }
+
+ DOMString onUnload = getAttribute(ATTR_ONUNLOAD);
+ if ( !onUnload.isNull() ) {
+ DocumentImpl* doc = contentDocument();
+ static_cast<HTMLDocumentImpl*>( 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<RenderPart*>( m_render );
- if(render->widget() && ::qt_cast<KHTMLView*>( render->widget()) )
+ if ( render->widget() && ::qt_cast<KHTMLView*>( render->widget()) ) {
return static_cast<KHTMLView*>( 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 <frame> and the
// first(IE)/last(Moz/Konq) registered onload on a <frameset>.
- // The body has the listener for <frame onload>
- 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 <frameset onload>
- 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<KHTMLView *>( 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<HTMLIFrameElementImpl \
*>(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<DOM::HTMLFrameElementImpl*>(RenderObject::element()); }
public slots:
- void slotViewCleared();
+ virtual void slotViewCleared();
+ virtual void slotDocCreated();
};
+// Used for HTMLIFrameElementImpl (<iframe>) and HTMLObjectBaseElement (<applet>, \
<embed>, <object>) // I can hardly call the class RenderObject ;-)
class RenderPartObject : public khtml::RenderPart
{
@@ -154,7 +157,8 @@ public:
virtual bool partLoadingErrorNotify( khtml::ChildFrame *childFrame, const KURL& \
url, const QString& serviceType );
public slots:
- void slotViewCleared();
+ virtual void slotDocCreated();
+ virtual void slotViewCleared();
private slots:
void slotPartLoadingErrorNotify();
};
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic