Reviewed by Darin.

	<rdar://problem/4002864> REGRESSION(125-146) getElementById in onload fails in a test case involving external resources

	Moved management of elementById hashtable from attach/detach to
	insertedIntoDocument/removedFromDocument, to avoid being thrown
	off by temporary detaches due to style recalcs.

        * khtml/xml/dom_elementimpl.cpp:
        (ElementImpl::insertedIntoDocument):
        (ElementImpl::removedFromDocument):
        (ElementImpl::attach):
        (ElementImpl::updateId):
        * khtml/xml/dom_elementimpl.h:

	Make sure that insertedIntoDocument is called before firing any
	DOM events.

        * khtml/xml/dom_nodeimpl.cpp:
        (NodeBaseImpl::dispatchChildInsertedEvents):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@8724 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog-2005-08-23 b/WebCore/ChangeLog-2005-08-23
index 41dda6c98d987c5bdafb651496fcb01a127d08f5..3b6fbec5d76876a4e659e2b4f5dd75b2efd39ea5 100644 (file)
--- a/WebCore/ChangeLog-2005-08-23
+++ b/WebCore/ChangeLog-2005-08-23
@@ -1,3 +1,26 @@
+2005-02-28  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Darin.
+
+       <rdar://problem/4002864> REGRESSION(125-146) getElementById in onload fails in a test case involving external resources
+        
+       Moved management of elementById hashtable from attach/detach to
+       insertedIntoDocument/removedFromDocument, to avoid being thrown
+       off by temporary detaches due to style recalcs.
+       
+        * khtml/xml/dom_elementimpl.cpp:
+        (ElementImpl::insertedIntoDocument):
+        (ElementImpl::removedFromDocument):
+        (ElementImpl::attach):
+        (ElementImpl::updateId):
+        * khtml/xml/dom_elementimpl.h:
+
+       Make sure that insertedIntoDocument is called before firing any
+       DOM events.
+       
+        * khtml/xml/dom_nodeimpl.cpp:
+        (NodeBaseImpl::dispatchChildInsertedEvents): 
+
 2005-02-28  David Hyatt  <hyatt@apple.com>
 
        Fix for 4028999, safari crashes when resetting if mallocsribble is on.  Clip rects were being cleared using dead

diff --git a/WebCore/khtml/xml/dom_elementimpl.cpp b/WebCore/khtml/xml/dom_elementimpl.cpp
index 33b988a3694eba2d10b68f2cad5c837b7cf0a64a..3bfeae68760a8d44c28cff4b1c0096a488ab3edd 100644 (file)
--- a/WebCore/khtml/xml/dom_elementimpl.cpp
+++ b/WebCore/khtml/xml/dom_elementimpl.cpp
@@ -408,12 +408,12 @@ RenderObject *ElementImpl::createRenderer(RenderArena *arena, RenderStyle *style
     return RenderObject::createObject(this, style);
 }
 
-void ElementImpl::attach()
+
+void ElementImpl::insertedIntoDocument()
 {
-#if SPEED_DEBUG < 1
-    createRendererIfNeeded();
-#endif
-    NodeBaseImpl::attach();
+    // need to do superclass processing first so inDocument() is true
+    // by the time we reach updateId
+    NodeBaseImpl::insertedIntoDocument();
 
     if (hasID()) {
         NamedAttrMapImpl *attrs = attributes(true);
@@ -426,7 +426,7 @@ void ElementImpl::attach()
     }
 }
 
-void ElementImpl::detach()
+void ElementImpl::removedFromDocument()
 {
     if (hasID()) {
         NamedAttrMapImpl *attrs = attributes(true);
@@ -438,7 +438,15 @@ void ElementImpl::detach()
         }
     }
 
-    NodeBaseImpl::detach();
+    NodeBaseImpl::removedFromDocument();
+}
+
+void ElementImpl::attach()
+{
+#if SPEED_DEBUG < 1
+    createRendererIfNeeded();
+#endif
+    NodeBaseImpl::attach();
 }
 
 void ElementImpl::recalcStyle( StyleChange change )
@@ -611,7 +619,7 @@ DOMString ElementImpl::toString() const
 
 void ElementImpl::updateId(const AtomicString& oldId, const AtomicString& newId)
 {
-    if (!attached())
+    if (!inDocument())
        return;
 
     if (oldId == newId)

diff --git a/WebCore/khtml/xml/dom_elementimpl.h b/WebCore/khtml/xml/dom_elementimpl.h
index 277e1d022fdec4ac1cd069014a89836c8fbd3155..d20b990e77b5221476893100e2281908e67c38b0 100644 (file)
--- a/WebCore/khtml/xml/dom_elementimpl.h
+++ b/WebCore/khtml/xml/dom_elementimpl.h
@@ -177,6 +177,8 @@ public:
     virtual NodeImpl *cloneNode ( bool deep );
     virtual DOMString nodeName() const;
     virtual bool isElementNode() const { return true; }
+    virtual void insertedIntoDocument();
+    virtual void removedFromDocument();
 
     // convenience methods which ignore exceptions
     void setAttribute (NodeImpl::Id id, const DOMString &value);
@@ -193,7 +195,6 @@ public:
     virtual QString state() { return QString::null; }
 
     virtual void attach();
-    virtual void detach();
     virtual khtml::RenderStyle *styleForRenderer(khtml::RenderObject *parent);
     virtual khtml::RenderObject *createRenderer(RenderArena *, khtml::RenderStyle *);
     virtual void recalcStyle( StyleChange = NoChange );
@@ -276,6 +277,7 @@ public:
     virtual Node removeNamedItem ( NodeImpl::Id id, int &exceptioncode );
     virtual Node setNamedItem ( NodeImpl* arg, int &exceptioncode );
 
+
     virtual AttrImpl *item ( unsigned long index ) const;
     unsigned long length() const { return len; }
 

diff --git a/WebCore/khtml/xml/dom_nodeimpl.cpp b/WebCore/khtml/xml/dom_nodeimpl.cpp
index 11d501cd3940aecaf23b4782ce4ba647748a66af..064262527b260c41bb1f107e66854f4bd6408fba 100644 (file)
--- a/WebCore/khtml/xml/dom_nodeimpl.cpp
+++ b/WebCore/khtml/xml/dom_nodeimpl.cpp
@@ -2205,6 +2205,16 @@ NodeImpl *NodeBaseImpl::childNode(unsigned long index)
 
 void NodeBaseImpl::dispatchChildInsertedEvents( NodeImpl *child, int &exceptioncode )
 {
+    NodeImpl *p = this;
+    while (p->parentNode())
+        p = p->parentNode();
+
+    if (p->nodeType() == Node::DOCUMENT_NODE) {
+        for (NodeImpl *c = child; c; c = c->traverseNextNode(child)) {
+            c->insertedIntoDocument();
+        }
+    }
+
     if (getDocument()->hasListenerType(DocumentImpl::DOMNODEINSERTED_LISTENER)) {
         child->dispatchEvent(new MutationEventImpl(EventImpl::DOMNODEINSERTED_EVENT,
                                                    true,false,this,DOMString(),DOMString(),DOMString(),0),exceptioncode,true);
@@ -2212,21 +2222,15 @@ void NodeBaseImpl::dispatchChildInsertedEvents( NodeImpl *child, int &exceptionc
             return;
     }
 
-    // dispatch the DOMNOdeInsertedInfoDocument event to all descendants
+    // dispatch the DOMNodeInsertedIntoDocument event to all descendants
     bool hasInsertedListeners = getDocument()->hasListenerType(DocumentImpl::DOMNODEINSERTEDINTODOCUMENT_LISTENER);
-    NodeImpl *p = this;
-    while (p->parentNode())
-        p = p->parentNode();
-    if (p->nodeType() == Node::DOCUMENT_NODE) {
-        for (NodeImpl *c = child; c; c = c->traverseNextNode(child)) {
-            c->insertedIntoDocument();
 
-            if (hasInsertedListeners) {
-                c->dispatchEvent(new MutationEventImpl(EventImpl::DOMNODEINSERTEDINTODOCUMENT_EVENT,
-                                                       false,false,0,DOMString(),DOMString(),DOMString(),0),exceptioncode,true);
-                if (exceptioncode)
-                    return;
-            }
+    if (hasInsertedListeners && p->nodeType() == Node::DOCUMENT_NODE) {
+        for (NodeImpl *c = child; c; c = c->traverseNextNode(child)) {
+            c->dispatchEvent(new MutationEventImpl(EventImpl::DOMNODEINSERTEDINTODOCUMENT_EVENT,
+                                                   false,false,0,DOMString(),DOMString(),DOMString(),0),exceptioncode,true);
+            if (exceptioncode)
+                return;
         }
     }
 }