[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    branches/KDE/4.0/kdelibs/khtml/ecma
From:       Maks Orlovich <maksim () kde ! org>
Date:       2008-03-14 18:52:22
Message-ID: 1205520742.197590.27811.nullmailer () svn ! kde ! org
[Download RAW message or body]

SVN commit 785693 by orlovich:

Apply a patch from Vyacheslav Tokarev (username tsjoker, hostname gmail, tld com)
that fixes multiple issues with event listeners:

1. Use the proper 'this' object for object-style event listeners.
2. Use the object, not function, for comparison of object-style
event listeners
3. Properly keep html-style and normal listener objects separate ---
they behave independent, even if they wrap the same function,
and of course have different event-cancellation semantics

BUG:147249
BUG:111568
BUG:128416


 M  +12 -8     kjs_events.cpp  
 M  +2 -2      kjs_events.h  
 M  +6 -6      kjs_window.cpp  
 M  +1 -1      kjs_window.h  


--- branches/KDE/4.0/kdelibs/khtml/ecma/kjs_events.cpp #785692:785693
@@ -48,14 +48,14 @@
 {
     //fprintf(stderr,"JSEventListener::JSEventListener this=%p \
listener=%p\n",this,listener.imp());  if (compareListenerImp) {
-    static_cast<Window*>(win.get())->jsEventListeners.insert(compareListenerImp, \
this); +    static_cast<Window*>(win.get())->jsEventListeners.insert(QPair<void*, \
bool>(compareListenerImp, html), this);  }
 }
 
 JSEventListener::~JSEventListener()
 {
   if (compareListenerImp) {
-    static_cast<Window*>(win.get())->jsEventListeners.remove(compareListenerImp);
+    static_cast<Window*>(win.get())->jsEventListeners.remove(QPair<void*, \
bool>(compareListenerImp, html));  }
   //fprintf(stderr,"JSEventListener::~JSEventListener this=%p \
listener=%p\n",this,listener.imp());  }
@@ -82,8 +82,14 @@
     List args;
     args.append(getDOMEvent(exec,evt.handle()));
 
-    // Set "this" to the event's current target
-    JSObject *thisObj = getDOMNode(exec,evt.currentTarget().handle())->getObject();
+    JSObject *thisObj = 0;
+    // Check whether handler is a function or an object with handleEvent method
+    if (listener == compareListenerImp) {
+      // Set "this" to the event's current target
+      thisObj = getDOMNode(exec,evt.currentTarget().handle())->getObject();
+    } else {
+      thisObj = compareListenerImp;
+    }
     if ( !thisObj ) {
       // Window events (window.onload/window.onresize etc.) must have 'this' set to \
                the window.
       // DocumentImpl::defaultEventHandler sets currentTarget to 0 to mean 'window'.
@@ -144,9 +150,6 @@
 
 JSLazyEventListener::~JSLazyEventListener()
 {
-  if (listener) {
-    static_cast<Window*>(win.get())->jsEventListeners.remove(listener);
-  }
 }
 
 void JSLazyEventListener::handleEvent(DOM::Event &evt)
@@ -186,6 +189,7 @@
       args.append(jsString(code));
       listener = constr->construct(exec, args, 
             Identifier(UString(name)), url, lineNum); // ### is globalExec ok ?
+      compareListenerImp = listener;
 
       if (exec->hadException()) {
         exec->clearException();
@@ -217,7 +221,7 @@
     code.clear();
 
     if (listener) {
-      static_cast<Window*>(win.get())->jsEventListeners.insert(listener,
+      static_cast<Window*>(win.get())->jsEventListeners.insert(QPair<void*, \
                bool>(compareListenerImp, true),
                                               (KJS::JSEventListener *)(this));
     }
 
--- branches/KDE/4.0/kdelibs/khtml/ecma/kjs_events.h #785692:785693
@@ -49,7 +49,7 @@
     // for Window::clear(). This is a bad hack though. The JSEventListener might not \
                get deleted
     // if it was added to a DOM node in another frame (#61467). But calling \
removeEventListener on  // all nodes we're listening to is quite difficult.
-    void clear() { listener = 0; }
+    void clear() { listener = 0; compareListenerImp = 0; }
     bool isHTMLEventListener() const { return html; }
 
   protected:
@@ -64,7 +64,7 @@
     // the imp() ptr of the 'passedListener' function _object_, as the \
                implementation will
     // call removeEventListener(.. [Object ..] on removal, and now we can \
                successfully lookup
     // the correct event listener, as well as the 'listener.handleEvent' function, \
                we need to call.
-    mutable JSObject *compareListenerImp;
+    mutable ProtectedPtr<JSObject> compareListenerImp;
     bool html;
     mutable ProtectedPtr<JSObject> win;
   };
--- branches/KDE/4.0/kdelibs/khtml/ecma/kjs_window.cpp #785692:785693
@@ -1393,6 +1393,7 @@
 
   // It's ObjectType, so it must be valid.
   JSObject *listenerObject = val->getObject();
+  JSObject *thisObject = listenerObject;
 
   // 'listener' is not a simple ecma function. (Always use sanity checks: Better \
safe than sorry!)  if (!listenerObject->implementsCall() && part && part->jScript() \
&& part->jScript()->interpreter()) @@ -1405,20 +1406,19 @@
 
     if(handleEventObject && handleEventObject->implementsCall())
     {
+      thisObject = listenerObject;
       listenerObject = handleEventObject;
     }
   }
 
-  JSEventListener *existingListener = jsEventListeners[listenerObject];
+  JSEventListener *existingListener = jsEventListeners[QPair<void*, \
bool>(thisObject, html)];  if (existingListener) {
-     if ( existingListener->isHTMLEventListener() != html )
-        // The existingListener will have the wrong type, so onclick= will behave \
                like addEventListener or vice versa.
-        kWarning() << "getJSEventListener: event listener already found but with \
html=" << !html << " - please report this, we thought it would never happen"; +    \
assert( existingListener->isHTMLEventListener() == html );  return existingListener;
   }
 
   // Note that the JSEventListener constructor adds it to our jsEventListeners list
-  return new JSEventListener(listenerObject, listenerObject, this, html);
+  return new JSEventListener(listenerObject, thisObject, this, html);
 }
 
 JSLazyEventListener *Window::getJSLazyEventListener(const QString& code, const \
QString& srcUrl, int line, @@ -1435,7 +1435,7 @@
   clearProperties();
 
   // Break the dependency between the listeners and their object
-  QHashIterator<void*, JSEventListener*> it(jsEventListeners);
+  QHashIterator<const QPair<void*, bool>, JSEventListener*> it(jsEventListeners);
   while ( it.hasNext() ) {
     it.next();
     it.value()->clear();
--- branches/KDE/4.0/kdelibs/khtml/ecma/kjs_window.h #785692:785693
@@ -135,7 +135,7 @@
     // Set the current "event" object
     void setCurrentEvent( DOM::EventImpl *evt );
 
-    QHash<void*, JSEventListener*> jsEventListeners;
+    QHash<const QPair<void*, bool>, JSEventListener*> jsEventListeners;
     virtual const ClassInfo* classInfo() const { return &info; }
     static const ClassInfo info;
     enum { Closed, Crypto, DefaultStatus, Status, Document, Node, EventCtor, Range,


[prev in list] [next in list] [prev in thread] [next in thread]