LayoutTests:
authormjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Feb 2006 23:07:54 +0000 (23:07 +0000)
committermjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Feb 2006 23:07:54 +0000 (23:07 +0000)
        Reviewed by Darin.

        Re-landing test case for:
        - various event cleanup, including fixing of the load event for iframes
        http://bugzilla.opendarwin.org/show_bug.cgi?id=7079

        * fast/events/iframe-object-onload-expected.txt: Added.
        * fast/events/iframe-object-onload.html: Added.

        - changed expected results and description for this test, unload should
        not fire in this case:

        * fast/events/onunload-body-property-expected.txt:
        * fast/events/onunload-body-property.html:

        - changed from UTF-16 to ASCII, there was no reason for this to be UTF-16.

        * fast/dom/attr_dead_doc.html:

WebCore:

        Reviewed by Darin.

        Re-landed the following with more fixes so it does not break tests:

        - various event cleanup, including fixing of the load event for iframes
        http://bugzilla.opendarwin.org/show_bug.cgi?id=7079

        Specific changes:

        - don't bother to nil-check the document, a node can never have a null document now
        - move temp event forgetting from dispatchEvent to dispatchGenericEvent
        - pass event down using RefPtr::release() to avoid ref thrashing
        - support default handlers even for non-bubbling events (only on
          target node) and skip calling default event handler explicitly in
          callers
        - dispatch a whole separate load event to a frame document's containing frame
        - don't let propagationStopped prevent this new event
        - remove bogus security check for iframe onload
        - dispatch window events on the document, not the body, and also for non-html
        - set onload, onunload, onbeforeunload from frameset tags on the window object
        - don't restrict load/unload events to HTML
        - send default handler to the dispatch object as previously

        * bridge/mac/MacFrame.mm:
        (WebCore::MacFrame::shouldClose):
        * khtml/html/html_baseimpl.cpp:
        (WebCore::HTMLFrameElementImpl::parseMappedAttribute):
        (WebCore::HTMLFrameSetElementImpl::parseMappedAttribute):
        * khtml/xml/DocumentImpl.cpp:
        (WebCore::DocumentImpl::implicitClose):
        * khtml/xml/NodeImpl.cpp:
        (WebCore::NodeImpl::dispatchEvent):
        (WebCore::NodeImpl::dispatchGenericEvent):
        (WebCore::NodeImpl::dispatchWindowEvent):
        * khtml/xml/NodeImpl.h:
        * page/Frame.cpp:
        (WebCore::Frame::stopLoading):
        (WebCore::Frame::setWindowHasFocus):

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/attr_dead_doc.html
LayoutTests/fast/events/iframe-object-onload-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/iframe-object-onload.html [new file with mode: 0644]
LayoutTests/fast/events/onunload-body-property-expected.txt
LayoutTests/fast/events/onunload-body-property.html
WebCore/ChangeLog
WebCore/bridge/mac/MacFrame.mm
WebCore/khtml/html/html_baseimpl.cpp
WebCore/khtml/xml/DocumentImpl.cpp
WebCore/khtml/xml/NodeImpl.cpp
WebCore/khtml/xml/NodeImpl.h
WebCore/page/Frame.cpp

index 86742f97ed95fa6d8889be4aa679b5e77732216b..9392348ab5a973d996bbffa01ee2b1b53b15729b 100644 (file)
@@ -1,3 +1,24 @@
+2006-02-05  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Darin.
+        
+        Re-landing test case for:
+        - various event cleanup, including fixing of the load event for iframes
+        http://bugzilla.opendarwin.org/show_bug.cgi?id=7079
+
+        * fast/events/iframe-object-onload-expected.txt: Added.
+        * fast/events/iframe-object-onload.html: Added.
+        
+        - changed expected results and description for this test, unload should
+        not fire in this case:
+        
+        * fast/events/onunload-body-property-expected.txt: 
+        * fast/events/onunload-body-property.html:
+        
+        - changed from UTF-16 to ASCII, there was no reason for this to be UTF-16.
+
+        * fast/dom/attr_dead_doc.html:
+
 2006-02-05  Darin Adler  <darin@apple.com>
 
         Reviewed by Maciej.
index 14b953f0871b54a3711155656e86c02683f20e27..b46c547ad9317bee1a531ef23836ff804a41cac0 100644 (file)
Binary files a/LayoutTests/fast/dom/attr_dead_doc.html and b/LayoutTests/fast/dom/attr_dead_doc.html differ
diff --git a/LayoutTests/fast/events/iframe-object-onload-expected.txt b/LayoutTests/fast/events/iframe-object-onload-expected.txt
new file mode 100644 (file)
index 0000000..bb0be3b
--- /dev/null
@@ -0,0 +1,26 @@
+This test checks that onload events fire on iframe and object elements when their contents finish loading, and that these are separate event objects from those that fire on the body inside the frame. If it passes, you will see 6 blocks below, each with a three-line message that ends in false.
+Fired load event on body inside iframe
+Fired event on iframe
+Outer event same as inner: false
+------
+Fired load event on body inside iframe
+Fired event on iframe even though default was prevented
+Outer event same as inner: false
+------
+Fired load event on body inside object
+Fired event on object even though propagation was stopped
+Outer event same as inner: false
+------
+Fired load event on body inside object
+Fired event on object
+Outer event same as inner: false
+------
+Fired load event on body inside object
+Fired event on object even though default was prevented
+Outer event same as inner: false
+------
+Fired load event on body inside object
+Fired event on object even though propagation was stopped
+Outer event same as inner: false
+------
+        
diff --git a/LayoutTests/fast/events/iframe-object-onload.html b/LayoutTests/fast/events/iframe-object-onload.html
new file mode 100644 (file)
index 0000000..555abfb
--- /dev/null
@@ -0,0 +1,68 @@
+<div> 
+This test checks that onload events fire on iframe and object
+elements when their contents finish loading, and that these are
+separate event objects from those that fire on the body inside the
+frame. If it passes, you will see 6 blocks below, each with a
+three-line message that ends in false.
+</div>
+<div id="console">
+</div>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function log(s)
+{
+    document.getElementById("console").innerHTML += s + "<br>";
+}
+
+var innerEvent = nil;
+
+function logOuter(msg, event)
+{
+    log(msg);
+    log('Outer event same as inner: ' + (innerEvent == event));
+    log('------');
+}
+
+function logInner(tag, event)
+{
+    parent.log("Fired load event on body inside " + tag); 
+    parent.innerEvent = event;
+}
+
+</script>
+
+<iframe 
+    src="data:text/html,<html><body onload='parent.logInner(%22iframe%22, event)'>Inner</body></html>" 
+    onload="logOuter('Fired event on iframe', event)">
+</iframe>
+
+<iframe 
+    src="data:text/html,<body onload='parent.logInner(%22iframe%22, event); event.preventDefault()'>preventDefault</body>" 
+    onload="logOuter('Fired event on iframe even though default was prevented', event)">
+</iframe>
+
+<iframe
+    src="data:text/html,<body onload='parent.logInner(%22object%22, event); event.stopPropagation()'>stopPropagation</body>" 
+    onload="logOuter('Fired event on object even though propagation was stopped', event)">
+</iframe>
+
+<object 
+    type="text/html"
+    data="data:text/html,<html><body onload='parent.logInner(%22object%22, event)'>Inner</body></html>" 
+    onload="logOuter('Fired event on object', event)">
+</object>
+
+<object 
+    type="text/html"
+    data="data:text/html,<body onload='parent.logInner(%22object%22, event); event.preventDefault()'>preventDefault</body>" 
+    onload="logOuter('Fired event on object even though default was prevented', event)">
+</object>
+
+<object 
+    type="text/html"
+    data="data:text/html,<body onload='parent.logInner(%22object%22, event); event.stopPropagation()'>stopPropagation</body>" 
+    onload="logOuter('Fired event on object even though propagation was stopped', event)">
+</object>
+
index 908c920c1392d11a136e2021a6499ba991baf096..f0eead708a7b1d4086995b4e7e91de5b7a181991 100644 (file)
@@ -1,2 +1 @@
-ALERT: unload
-You should have seen an unload alert appear.
+You should NOT have seen an unload alert appear. This test attaches an unload handler via document.body.onunload. This works in Internet Explorer because JS properties are linked directly to HTML attributes, and the onunload attribute has this behavior. But it should not work in browsers that don't have this quirk.
index afeb869c2a69bb9bbdf6490001168c043716ce2c..4e376f321946a24262669af82d1d0cd92b5fc622 100644 (file)
@@ -16,7 +16,7 @@ function unload()
 function load()
 {
     document.body.onunload = unload;
-    location = "data:text/html,You should have seen an unload alert appear.<script>if (window.layoutTestController) layoutTestController.notifyDone(); </" + "script>";
+    location = "data:text/html,You should NOT have seen an unload alert appear. This test attaches an unload handler via document.body.onunload. This works in Internet Explorer because JS properties are linked directly to HTML attributes, and the onunload attribute has this behavior. But it should not work in browsers that don't have this quirk.<script>if (window.layoutTestController) layoutTestController.notifyDone(); </" + "script>";
 }
 
 </script>
index ddfdb1ff5e450bc8da9cc6f1f9ab5bcda3297918..af120f038e2bd830ceac2a2821ba75f586dc8c3b 100644 (file)
@@ -1,3 +1,44 @@
+2006-02-05  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Darin.
+        
+        Re-landed the following with more fixes so it does not break tests:
+
+        - various event cleanup, including fixing of the load event for iframes
+        http://bugzilla.opendarwin.org/show_bug.cgi?id=7079
+        
+        Specific changes:
+        
+        - don't bother to nil-check the document, a node can never have a null document now
+        - move temp event forgetting from dispatchEvent to dispatchGenericEvent
+        - pass event down using RefPtr::release() to avoid ref thrashing
+        - support default handlers even for non-bubbling events (only on
+          target node) and skip calling default event handler explicitly in
+          callers
+        - dispatch a whole separate load event to a frame document's containing frame
+        - don't let propagationStopped prevent this new event
+        - remove bogus security check for iframe onload
+        - dispatch window events on the document, not the body, and also for non-html
+        - set onload, onunload, onbeforeunload from frameset tags on the window object
+        - don't restrict load/unload events to HTML
+        - send default handler to the dispatch object as previously
+
+        * bridge/mac/MacFrame.mm:
+        (WebCore::MacFrame::shouldClose):
+        * khtml/html/html_baseimpl.cpp:
+        (WebCore::HTMLFrameElementImpl::parseMappedAttribute):
+        (WebCore::HTMLFrameSetElementImpl::parseMappedAttribute):
+        * khtml/xml/DocumentImpl.cpp:
+        (WebCore::DocumentImpl::implicitClose):
+        * khtml/xml/NodeImpl.cpp:
+        (WebCore::NodeImpl::dispatchEvent):
+        (WebCore::NodeImpl::dispatchGenericEvent):
+        (WebCore::NodeImpl::dispatchWindowEvent):
+        * khtml/xml/NodeImpl.h:
+        * page/Frame.cpp:
+        (WebCore::Frame::stopLoading):
+        (WebCore::Frame::setWindowHasFocus):
+
 2006-02-05  Darin Adler  <darin@apple.com>
 
         Reviewed by Geoff.
index 233ed48f672b5a9346a61006cbaea58cd22d684e..4c960ae7d2f1f54042ba3ca8823652cd63c243ca 100644 (file)
@@ -3531,7 +3531,7 @@ bool MacFrame::shouldClose()
     RefPtr<BeforeUnloadEventImpl> event = new BeforeUnloadEventImpl;
     event->setTarget(doc.get());
     int exception = 0;
-    body->dispatchGenericEvent(event, exception);
+    body->dispatchGenericEvent(event, exception, true);
     if (!event->defaultPrevented() && doc)
         doc->defaultEventHandler(event.get());
     if (event->result().isNull())
index ef13c09c0cc635acb328257778ca2559cc740eb9..e7d4c2d8fc5f55d6b5ce34faf890b6db639d0f85 100644 (file)
@@ -402,6 +402,7 @@ void HTMLFrameElementImpl::parseMappedAttribute(MappedAttributeImpl *attr)
     } else if (attr->name() == onloadAttr) {
         setHTMLEventListener(loadEvent, attr);
     } else if (attr->name() == onbeforeunloadAttr) {
+        // FIXME: should <frame> elements have beforeunload handlers?
         setHTMLEventListener(beforeunloadEvent, attr);
     } else if (attr->name() == onunloadAttr) {
         setHTMLEventListener(unloadEvent, attr);
@@ -704,11 +705,11 @@ void HTMLFrameSetElementImpl::parseMappedAttribute(MappedAttributeImpl *attr)
         if(!m_border)
             frameborder = false;
     } else if (attr->name() == onloadAttr) {
-        setHTMLEventListener(loadEvent, attr);
+        getDocument()->setHTMLWindowEventListener(loadEvent, attr);
     } else if (attr->name() == onbeforeunloadAttr) {
-        setHTMLEventListener(beforeunloadEvent, attr);
+        getDocument()->setHTMLWindowEventListener(beforeunloadEvent, attr);
     } else if (attr->name() == onunloadAttr) {
-        setHTMLEventListener(unloadEvent, attr);
+        getDocument()->setHTMLWindowEventListener(unloadEvent, attr);
     } else
         HTMLElementImpl::parseMappedAttribute(attr);
 }
index 8977cd7f5d27aaced449b45d44bb9736a9038e40..2b85b070979b068764303f2e10fc4b2e7821be3c 100644 (file)
@@ -1189,20 +1189,14 @@ void DocumentImpl::implicitClose()
         }
     }
     
-    NodeImpl *onloadTarget = body;
-    if (!isHTMLDocument())
-        onloadTarget = documentElement();
-
-    if (onloadTarget) {
-        dispatchImageLoadEventsNow();
-        onloadTarget->dispatchWindowEvent(loadEvent, false, false);
-        if (Frame *p = frame())
-            p->handledOnloadEvents();
+    dispatchImageLoadEventsNow();
+    this->dispatchWindowEvent(loadEvent, false, false);
+    if (Frame *p = frame())
+        p->handledOnloadEvents();
 #ifdef INSTRUMENT_LAYOUT_SCHEDULING
-        if (!ownerElement())
-            printf("onload fired at %d\n", elapsedTime());
+    if (!ownerElement())
+        printf("onload fired at %d\n", elapsedTime());
 #endif
-    }
 
     m_processingLoadEvent = false;
 
index 47216bcb1ef577fb2b0a2f116ee00423268dedab..6361a4e2e2fa8484f37bddb1109fc1acd9b5d572 100644 (file)
@@ -495,31 +495,12 @@ bool NodeImpl::dispatchEvent(PassRefPtr<EventImpl> e, int &exceptioncode, bool t
     }
     evt->setTarget(this);
 
-    // We've had at least one report of a crash on a page where document is null here.
-    // Unfortunately that page no longer exists, but we'll make this code robust against
-    // that anyway.
-    // FIXME: Much code in this class assumes document is non-null; it would be better to
-    // ensure that document can never be null.
-    Frame *frame = 0;
-    RefPtr<FrameView> view;
-
-    if (DocumentImpl *doc = getDocument()) {
-        frame = doc->frame();
-        view = doc->view();
-    }
-
-    bool ret = dispatchGenericEvent(evt, exceptioncode);
+    RefPtr<FrameView> view = getDocument()->view();
 
-    // If tempEvent is true, this means that the DOM implementation will not be storing a reference to the event, i.e.
-    // there is no way to retrieve it from javascript if a script does not already have a reference to it in a variable.
-    // So there is no need for the interpreter to keep the event in it's cache
-    if (tempEvent && frame && frame->jScript())
-        frame->jScript()->finishedWithEvent(evt.get());
-
-    return ret;
+    return dispatchGenericEvent(evt.release(), exceptioncode, tempEvent);
 }
 
-bool NodeImpl::dispatchGenericEvent(PassRefPtr<EventImpl> e, int &/*exceptioncode */)
+bool NodeImpl::dispatchGenericEvent(PassRefPtr<EventImpl> e, int &/*exceptioncode */, bool tempEvent)
 {
     RefPtr<EventImpl> evt(e);
     assert(!eventDispatchForbidden());
@@ -535,7 +516,6 @@ bool NodeImpl::dispatchGenericEvent(PassRefPtr<EventImpl> e, int &/*exceptioncod
         nodeChain.prepend(n);
     }
 
-    
     QPtrListIterator<NodeImpl> it(nodeChain);
     
     // Before we begin dispatching events, give the target node a chance to do some work prior
@@ -589,13 +569,15 @@ bool NodeImpl::dispatchGenericEvent(PassRefPtr<EventImpl> e, int &/*exceptioncod
     // Now call the post dispatch.
     postDispatchEventHandler(evt.get(), data);
     
-    if (evt->bubbles()) {
-        // now we call all default event handlers (this is not part of DOM - it is internal to khtml)
+    // now we call all default event handlers (this is not part of DOM - it is internal to khtml)
 
-        it.toLast();
+    it.toLast();
+    if (evt->bubbles())
         for (; it.current() && !evt->defaultPrevented() && !evt->defaultHandled(); --it)
             it.current()->defaultEventHandler(evt.get());
-    }
+    else
+        if (!evt->defaultPrevented() && !evt->defaultHandled())
+            it.current()->defaultEventHandler(evt.get());
     
     // deref all nodes in chain
     it.toFirst();
@@ -604,6 +586,15 @@ bool NodeImpl::dispatchGenericEvent(PassRefPtr<EventImpl> e, int &/*exceptioncod
 
     DocumentImpl::updateDocumentsRendering();
 
+    // If tempEvent is true, this means that the DOM implementation
+    // will not be storing a reference to the event, i.e.  there is no
+    // way to retrieve it from javascript if a script does not already
+    // have a reference to it in a variable.  So there is no need for
+    // the interpreter to keep the event in it's cache
+    Frame *frame = getDocument()->frame();
+    if (tempEvent && frame && frame->jScript())
+        frame->jScript()->finishedWithEvent(evt.get());
+
     return !evt->defaultPrevented(); // ### what if defaultPrevented was called before dispatchEvent?
 }
 
@@ -621,29 +612,17 @@ bool NodeImpl::dispatchWindowEvent(const AtomicString &eventType, bool canBubble
     RefPtr<EventImpl> evt = new EventImpl(eventType, canBubbleArg, cancelableArg);
     RefPtr<DocumentImpl> doc = getDocument();
     evt->setTarget(doc.get());
-    bool r = dispatchGenericEvent(evt, exceptioncode);
-    if (!evt->defaultPrevented() && doc)
-        doc->defaultEventHandler(evt.get());
+    bool r = dispatchGenericEvent(evt.release(), exceptioncode, true);
 
-    if (eventType == loadEvent && !evt->propagationStopped() && doc) {
-        // For onload events, send them to the enclosing frame only.
+    if (eventType == loadEvent) {
+        // For onload events, send a separate load event to the enclosing frame only.
         // This is a DOM extension and is independent of bubbling/capturing rules of
-        // the DOM.  You send the event only to the enclosing frame.  It does not
-        // bubble through the parent document.
-        ElementImpl* elt = doc->ownerElement();
-        if (elt && (elt->getDocument()->domain().isNull() ||
-                    elt->getDocument()->domain() == doc->domain())) {
-            // We also do a security check, since we don't want to allow the enclosing
-            // iframe to see loads of child documents in other domains.
-            evt->setCurrentTarget(elt);
-
-            // Capturing first.
-            elt->handleLocalEvents(evt.get(), true);
-
-            // Bubbling second.
-            if (!evt->propagationStopped())
-                elt->handleLocalEvents(evt.get(), false);
-            r = !evt->defaultPrevented();
+        // the DOM.
+        ElementImpl* ownerElement = doc->ownerElement();
+        if (ownerElement) {
+            RefPtr<EventImpl> ownerEvent = new EventImpl(eventType, false, cancelableArg);
+            ownerEvent->setTarget(ownerElement);
+            ownerElement->dispatchGenericEvent(ownerEvent.release(), exceptioncode, true);
         }
     }
 
index 2df640f96d0202ed9178880a7059d02db8fba37f..8f153b687c8fe1e5f173a2d1921939edde899f30 100644 (file)
@@ -272,7 +272,7 @@ public:
     void removeAllEventListeners();
 
     bool dispatchEvent(PassRefPtr<EventImpl>, ExceptionCode&, bool tempEvent = false);
-    bool dispatchGenericEvent(PassRefPtr<EventImpl>, ExceptionCode&);
+    bool dispatchGenericEvent(PassRefPtr<EventImpl>, ExceptionCode&, bool tempEvent = false);
     bool dispatchHTMLEvent(const AtomicString& eventType, bool canBubble, bool cancelable);
     bool dispatchWindowEvent(const AtomicString& eventType, bool canBubble, bool cancelable);
     bool dispatchMouseEvent(QMouseEvent*, const AtomicString& overrideType,
index 0a829e14017afeaa1bd3e8986877bf80b7e9ce91..22e4db483f81f61c41c23ee4f5da0982c09645d3 100644 (file)
@@ -325,12 +325,10 @@ void Frame::stopLoading(bool sendUnload)
   }
 
   if (sendUnload) {
-    if ( d->m_doc && d->m_doc->isHTMLDocument() ) {
-      HTMLDocumentImpl* hdoc = static_cast<HTMLDocumentImpl*>( d->m_doc );
-      
-      if ( hdoc->body() && d->m_bLoadEventEmitted && !d->m_bUnloadEventEmitted ) {
-        hdoc->body()->dispatchWindowEvent( unloadEvent, false, false );
-        if ( d->m_doc )
+    if (d->m_doc) {
+      if (d->m_bLoadEventEmitted && !d->m_bUnloadEventEmitted ) {
+        d->m_doc->dispatchWindowEvent(unloadEvent, false, false);
+        if (d->m_doc)
           d->m_doc->updateRendering();
         d->m_bUnloadEventEmitted = true;
       }
@@ -3591,8 +3589,7 @@ void Frame::setWindowHasFocus(bool flag)
     m_windowHasFocus = flag;
     
     if (DocumentImpl *doc = document())
-        if (NodeImpl *body = doc->body())
-            body->dispatchWindowEvent(flag ? focusEvent : blurEvent, false, false);
+        doc->dispatchWindowEvent(flag ? focusEvent : blurEvent, false, false);
 }
 
 QChar Frame::backslashAsCurrencySymbol() const