patch: fix for event.cancelBubble and event.returnValue

Maciej Stachowiak mjs at apple.com
Tue Oct 28 17:31:13 CET 2003


The most important part of this is ensuring that cancelBubble = true 
does not prevent default. A lot of sites apparently use cancelBubble to 
eat the event but still alow default handling, as for a link click.

-------------- next part --------------
Index: ChangeLog
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/ChangeLog,v
retrieving revision 1.2140
diff -u -p -r1.2140 ChangeLog
--- ChangeLog	2003/10/29 00:00:35	1.2140
+++ ChangeLog	2003/10/29 01:15:57
@@ -1,3 +1,32 @@
+2003-10-28  Maciej Stachowiak  <mjs at apple.com>
+
+        Reviewed by Chris.
+
+	- fixed 3379725 - for <a> elements with both onclick and href, Web Kit's behavior does not match other browsers
+
+	The fix for this was to make cancelBubble only prevent bubbling,
+	but not stop propagation or prevent default. While I was at it, I
+	made returnValue work right too.
+	
+        * khtml/ecma/kjs_events.cpp:
+        (DOMEvent::getValueProperty): Implement cancelBubble and returnValue properties properly.
+        (DOMEvent::putValue): Ditto.
+        * khtml/dom/dom2_events.cpp:
+        (Event::setCancelBubble): Call impl.
+        (Event::setDefaultPrevented): Ditto.
+        (Event::getCancelBubble): Ditto.
+        (Event::defaultPrevented): Ditto.
+        * khtml/dom/dom2_events.h:
+        * khtml/xml/dom2_eventsimpl.h:
+        (DOM::EventImpl::propagationStopped): Made this a const method.
+        (DOM::EventImpl::defaultPrevented): Ditto.
+        (DOM::EventImpl::setCancelBubble): Implemented by setting a new field.
+        (DOM::EventImpl::getCancelBubble): Corresponding getter.
+        (DOM::EventImpl::setDefaultPrevented): Set the already existing field for this.
+        * khtml/xml/dom_nodeimpl.cpp:
+        (NodeImpl::dispatchGenericEvent): Check getCancelBubble() when bubbling. Do not
+	check bubbles() before default handling.
+
 2003-10-28  Chris Blumenberg  <cblu at apple.com>
 
 	Fixed: <rdar://problem/3464472>: REGRESSION: New CSS cursor support breaks style="cursor:default
Index: khtml/dom/dom2_events.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/dom/dom2_events.cpp,v
retrieving revision 1.12
diff -u -p -r1.12 khtml/dom/dom2_events.cpp
--- khtml/dom/dom2_events.cpp	2003/10/20 17:28:09	1.12
+++ khtml/dom/dom2_events.cpp	2003/10/29 01:15:57
@@ -177,6 +177,38 @@ DOMString Event::eventModuleName()
     return impl->eventModuleName();
 }
 
+void Event::setCancelBubble(bool cancel)
+{
+    if (!impl)
+	throw DOMException(DOMException::INVALID_STATE_ERR);
+
+    impl->setCancelBubble(cancel);
+}
+
+void Event::setDefaultPrevented(bool defaultPrevented)
+{
+    if (!impl)
+	throw DOMException(DOMException::INVALID_STATE_ERR);
+
+    impl->setDefaultPrevented(defaultPrevented);
+}
+ 
+bool Event::getCancelBubble() const
+{
+    if (!impl)
+	throw DOMException(DOMException::INVALID_STATE_ERR);
+
+    return impl->getCancelBubble();
+}
+
+bool Event::defaultPrevented() const
+{
+    if (!impl)
+	throw DOMException(DOMException::INVALID_STATE_ERR);
+    
+    return impl->defaultPrevented();
+}
+
 // -----------------------------------------------------------------------------
 
 #ifndef SAVE_SPACE
Index: khtml/dom/dom2_events.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/dom/dom2_events.h,v
retrieving revision 1.11
diff -u -p -r1.11 khtml/dom/dom2_events.h
--- khtml/dom/dom2_events.h	2003/10/20 17:28:09	1.11
+++ khtml/dom/dom2_events.h	2003/10/29 01:15:57
@@ -215,7 +215,6 @@ public:
      */
     void preventDefault();
 
-
     /**
      * The initEvent method is used to initialize the value of an Event created
      * through the DocumentEvent interface. This method may only be called
@@ -258,6 +257,12 @@ public:
      * Document::createEvent() (e.g. UIEvents)
      */
     DOMString eventModuleName();
+
+    /* Nonstandard extensions needed to support widely used JS event properties */
+    void setCancelBubble(bool cancel);
+    void setDefaultPrevented(bool returnValue);
+    bool getCancelBubble() const;
+    bool defaultPrevented() const;
 
 protected:
     Event(EventImpl *i);
Index: khtml/ecma/kjs_events.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/ecma/kjs_events.cpp,v
retrieving revision 1.17
diff -u -p -r1.17 khtml/ecma/kjs_events.cpp
--- khtml/ecma/kjs_events.cpp	2003/05/13 21:19:56	1.17
+++ khtml/ecma/kjs_events.cpp	2003/10/29 01:15:57
@@ -237,8 +237,11 @@ Value DOMEvent::getValueProperty(ExecSta
   case EventPhase:
     return Number((unsigned int)event.eventPhase());
   case Bubbles:
-  case CancelBubble: // MSIE extension. not sure if readable. and returnValue ?
     return Boolean(event.bubbles());
+  case CancelBubble:
+    return Boolean(event.getCancelBubble());
+  case ReturnValue:
+    return Boolean(!event.defaultPrevented());
   case Cancelable:
     return Boolean(event.cancelable());
   case TimeStamp:
@@ -260,12 +263,10 @@ void DOMEvent::putValue(ExecState *exec,
 {
   switch (token) {
   case ReturnValue:
-    if (value.toBoolean(exec))
-      event.preventDefault();
+    event.setDefaultPrevented(!value.toBoolean(exec));
     break;
   case CancelBubble:
-    if (value.toBoolean(exec))
-      event.stopPropagation();
+    event.setCancelBubble(value.toBoolean(exec));
     break;
   default:
     break;
Index: khtml/xml/dom2_eventsimpl.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/xml/dom2_eventsimpl.h,v
retrieving revision 1.9
diff -u -p -r1.9 khtml/xml/dom2_eventsimpl.h
--- khtml/xml/dom2_eventsimpl.h	2003/10/20 17:28:10	1.9
+++ khtml/xml/dom2_eventsimpl.h	2003/10/29 01:15:57
@@ -119,8 +119,8 @@ public:
     virtual bool isKeyboardEvent() { return false; }
     virtual DOMString eventModuleName() { return ""; }
 
-    virtual bool propagationStopped() { return m_propagationStopped; }
-    virtual bool defaultPrevented() { return m_defaultPrevented; }
+    virtual bool propagationStopped() const { return m_propagationStopped; }
+    virtual bool defaultPrevented() const { return m_defaultPrevented; }
 
     static EventId typeToId(DOMString type);
     static DOMString idToType(EventId id);
@@ -128,6 +128,10 @@ public:
     virtual void setDefaultHandled();
     bool defaultHandled() const { return m_defaultHandled; }
 
+    void setCancelBubble(bool cancel) { m_cancelBubble = cancel; }
+    void setDefaultPrevented(bool defaultPrevented) { m_defaultPrevented = defaultPrevented; }
+    bool getCancelBubble() const { return m_cancelBubble; }
+
 protected:
     DOMStringImpl *m_type;
     bool m_canBubble;
@@ -136,6 +140,8 @@ protected:
     bool m_propagationStopped;
     bool m_defaultPrevented;
     bool m_defaultHandled;
+    bool m_cancelBubble;
+
     EventId m_id;
     NodeImpl *m_currentTarget; // ref > 0 maintained externally
     unsigned short m_eventPhase;
@@ -164,6 +170,7 @@ public:
 		     long detailArg);
     virtual bool isUIEvent() { return true; }
     virtual DOMString eventModuleName() { return "UIEvents"; }
+
 protected:
     AbstractViewImpl *m_view;
     long m_detail;
Index: khtml/xml/dom_nodeimpl.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/xml/dom_nodeimpl.cpp,v
retrieving revision 1.42
diff -u -p -r1.42 khtml/xml/dom_nodeimpl.cpp
--- khtml/xml/dom_nodeimpl.cpp	2003/10/23 22:23:25	1.42
+++ khtml/xml/dom_nodeimpl.cpp	2003/10/29 01:15:57
@@ -547,7 +547,7 @@ bool NodeImpl::dispatchGenericEvent( Eve
 
     if (evt->bubbles()) {
         evt->setEventPhase(Event::BUBBLING_PHASE);
-        for (; it.current() && !evt->propagationStopped(); --it) {
+        for (; it.current() && !evt->propagationStopped() && !evt->getCancelBubble(); --it) {
             evt->setCurrentTarget(it.current());
             it.current()->handleLocalEvents(evt,false);
         }
@@ -556,13 +556,12 @@ bool NodeImpl::dispatchGenericEvent( Eve
     evt->setCurrentTarget(0);
     evt->setEventPhase(0); // I guess this is correct, the spec does not seem to say
                            // anything about the default event handler phase.
-    if (evt->bubbles()) {
-        // now we call all default event handlers (this is not part of DOM - it is internal to khtml)
 
-        it.toLast();
-        for (; it.current() && !evt->propagationStopped() && !evt->defaultPrevented() && !evt->defaultHandled(); --it)
-                    it.current()->defaultEventHandler(evt);
-    }
+    // now we call all default event handlers (this is not part of DOM - it is internal to khtml)
+
+    it.toLast();
+    for (; it.current() && !evt->propagationStopped() && !evt->defaultPrevented() && !evt->defaultHandled(); --it)
+	it.current()->defaultEventHandler(evt);
 
     // In the case of a mouse click, also send a DOMActivate event, which causes things like form submissions
     // to occur. Note that this only happens for _real_ mouse clicks (for which we get a KHTML_CLICK_EVENT or


More information about the Khtml-devel mailing list