Title: [100974] trunk
Revision
100974
Author
commit-qu...@webkit.org
Date
2011-11-21 18:59:03 -0800 (Mon, 21 Nov 2011)

Log Message

If an event listener is a function, it should be called and not checked for handleEvent.
This also covers callbacks, which follow the same spec but are implemented separately.
https://bugs.webkit.org/show_bug.cgi?id=62518

Patch by Rob Brackett <r...@robbrackett.com> on 2011-11-21
Reviewed by Sam Weinig.

Source/WebCore:

Tests: fast/events/dispatch-to-function-with-handle-event.html
       fast/js/callback-function-with-handle-event.html

* bindings/js/JSCallbackData.cpp:
(WebCore::JSCallbackData::invokeCallback):
* bindings/js/JSEventListener.cpp:
(WebCore::JSEventListener::handleEvent):

LayoutTests:

* fast/events/dispatch-to-function-with-handle-event-expected.txt: Added.
* fast/events/dispatch-to-function-with-handle-event.html: Added.
* fast/js/callback-function-with-handle-event-expected.txt: Added.
* fast/js/callback-function-with-handle-event.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (100973 => 100974)


--- trunk/LayoutTests/ChangeLog	2011-11-22 02:56:00 UTC (rev 100973)
+++ trunk/LayoutTests/ChangeLog	2011-11-22 02:59:03 UTC (rev 100974)
@@ -1,3 +1,16 @@
+2011-11-21  Rob Brackett  <r...@robbrackett.com>
+
+        If an event listener is a function, it should be called and not checked for handleEvent.
+        This also covers callbacks, which follow the same spec but are implemented separately.
+        https://bugs.webkit.org/show_bug.cgi?id=62518
+
+        Reviewed by Sam Weinig.
+
+        * fast/events/dispatch-to-function-with-handle-event-expected.txt: Added.
+        * fast/events/dispatch-to-function-with-handle-event.html: Added.
+        * fast/js/callback-function-with-handle-event-expected.txt: Added.
+        * fast/js/callback-function-with-handle-event.html: Added.
+
 2011-11-21  Filip Pizlo  <fpi...@apple.com>
 
         Should have a test for corner cases of DFG's integer optimization and CSE.

Added: trunk/LayoutTests/fast/events/dispatch-to-function-with-handle-event-expected.txt (0 => 100974)


--- trunk/LayoutTests/fast/events/dispatch-to-function-with-handle-event-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/dispatch-to-function-with-handle-event-expected.txt	2011-11-22 02:59:03 UTC (rev 100974)
@@ -0,0 +1,4 @@
+When the listener passed to addEventListener() is a function, it should be called when the event occurs. The listener function should not be tested for adherence to the EventListener interface and have its handleEvent() method called if present.
+
+PASS The listener function should have been called to handle the event.
+

Added: trunk/LayoutTests/fast/events/dispatch-to-function-with-handle-event.html (0 => 100974)


--- trunk/LayoutTests/fast/events/dispatch-to-function-with-handle-event.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/dispatch-to-function-with-handle-event.html	2011-11-22 02:59:03 UTC (rev 100974)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+    <title>Dispatching to a Function Object Implementing handleEvent()</title>
+  </head>
+  <body>
+    
+    <p>
+      When the listener passed to <code>addEventListener()</code> is a function, it should be called when the event occurs.
+      The listener function should <em>not</em> be tested for adherence to the <code>EventListener</code> interface 
+      and have its <code>handleEvent()</code> method called if present.
+    </p>
+
+    <p id="console"></p>
+    
+    <script src=""
+    <script type="text/_javascript_" charset="utf-8">
+      // This function should be called.
+      var listener = function(event) {
+        testPassed("The listener function should have been called to handle the event.");
+      };
+      // This function should not be called.
+      listener.handleEvent = function(event) {
+        testFailed("The listener function should have been called to handle the event.");
+      };
+      
+      // Send the test event
+      window.addEventListener("testevent", listener, false);
+      var event = document.createEvent("Event");
+      event.initEvent("testevent", true, true);
+      window.dispatchEvent(event);
+    </script>
+  
+  </body>
+</html>
\ No newline at end of file

Added: trunk/LayoutTests/fast/js/callback-function-with-handle-event-expected.txt (0 => 100974)


--- trunk/LayoutTests/fast/js/callback-function-with-handle-event-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/js/callback-function-with-handle-event-expected.txt	2011-11-22 02:59:03 UTC (rev 100974)
@@ -0,0 +1,7 @@
+When a JS callback is a function, it should be called. If the function has another function as its handleEvent property, that function should not be called.
+
+PASS The callback function was called directly.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/js/callback-function-with-handle-event.html (0 => 100974)


--- trunk/LayoutTests/fast/js/callback-function-with-handle-event.html	                        (rev 0)
+++ trunk/LayoutTests/fast/js/callback-function-with-handle-event.html	2011-11-22 02:59:03 UTC (rev 100974)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+    <title>Callback Function Objects Implementing handleEvent()</title>
+  </head>
+  <body>
+    
+    <p>
+      When a JS callback is a function, it should be called. If the function has another function as 
+      its <code>handleEvent</code> property, that function should <em>not</em> be called.
+    </p>
+
+    <p id="console"></p>
+    
+    <script src=""
+    <script type="text/_javascript_" charset="utf-8">
+      window.jsTestIsAsync = true;
+      
+      // This function should be called.
+      var callback = function(event) {
+        testPassed("The callback function was called directly.");
+        finishJSTest();
+      };
+      // This function should not be called.
+      callback.handleEvent = function(event) {
+        testFailed("The callback function's handleEvent property was called instead of the function itself.");
+        finishJSTest();
+      };
+      
+      // Database is one of several uses of JS Callbacks
+      var db = openDatabase("callback-function-with-handle-event-test", "", "Test for callback functions that implement a handleEvent() method.", 1);
+      db.changeVersion(db.version, "1.0", callback);
+    </script>
+    <script src=""
+  
+  </body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (100973 => 100974)


--- trunk/Source/WebCore/ChangeLog	2011-11-22 02:56:00 UTC (rev 100973)
+++ trunk/Source/WebCore/ChangeLog	2011-11-22 02:59:03 UTC (rev 100974)
@@ -1,3 +1,19 @@
+2011-11-21  Rob Brackett  <r...@robbrackett.com>
+        
+        If an event listener is a function, it should be called and not checked for handleEvent.
+        This also covers callbacks, which follow the same spec but are implemented separately.
+        https://bugs.webkit.org/show_bug.cgi?id=62518
+
+        Reviewed by Sam Weinig.
+
+        Tests: fast/events/dispatch-to-function-with-handle-event.html
+               fast/js/callback-function-with-handle-event.html
+
+        * bindings/js/JSCallbackData.cpp:
+        (WebCore::JSCallbackData::invokeCallback):
+        * bindings/js/JSEventListener.cpp:
+        (WebCore::JSEventListener::handleEvent):
+
 2011-11-21  Rakesh KN  <rakesh...@motorola.com>
 
         Need support for dirname attribute

Modified: trunk/Source/WebCore/bindings/js/JSCallbackData.cpp (100973 => 100974)


--- trunk/Source/WebCore/bindings/js/JSCallbackData.cpp	2011-11-22 02:56:00 UTC (rev 100973)
+++ trunk/Source/WebCore/bindings/js/JSCallbackData.cpp	2011-11-22 02:59:03 UTC (rev 100974)
@@ -48,15 +48,15 @@
     ASSERT(globalObject());
 
     ExecState* exec = globalObject()->globalExec();
-    JSValue function = callback()->get(exec, Identifier(exec, "handleEvent"));
+    JSValue function = callback();
 
     CallData callData;
-    CallType callType = getCallData(function, callData);
+    CallType callType = callback()->methodTable()->getCallData(callback(), callData);
     if (callType == CallTypeNone) {
-        callType = callback()->methodTable()->getCallData(callback(), callData);
+        function = callback()->get(exec, Identifier(exec, "handleEvent"));
+        callType = getCallData(function, callData);
         if (callType == CallTypeNone)
             return JSValue();
-        function = callback();
     }
     
     globalObject()->globalData().timeoutChecker.start();

Modified: trunk/Source/WebCore/bindings/js/JSEventListener.cpp (100973 => 100974)


--- trunk/Source/WebCore/bindings/js/JSEventListener.cpp	2011-11-22 02:56:00 UTC (rev 100973)
+++ trunk/Source/WebCore/bindings/js/JSEventListener.cpp	2011-11-22 02:59:03 UTC (rev 100974)
@@ -95,13 +95,14 @@
     }
 
     ExecState* exec = globalObject->globalExec();
-    JSValue handleEventFunction = jsFunction->get(exec, Identifier(exec, "handleEvent"));
+    JSValue handleEventFunction = jsFunction;
 
     CallData callData;
     CallType callType = getCallData(handleEventFunction, callData);
+    // If jsFunction is not actually a function, see if it implements the EventListener interface and use that
     if (callType == CallTypeNone) {
-        handleEventFunction = JSValue();
-        callType = jsFunction->methodTable()->getCallData(jsFunction, callData);
+        handleEventFunction = jsFunction->get(exec, Identifier(exec, "handleEvent"));
+        callType = getCallData(handleEventFunction, callData);
     }
 
     if (callType != CallTypeNone) {
@@ -117,17 +118,10 @@
         DynamicGlobalObjectScope globalObjectScope(globalData, globalData.dynamicGlobalObject ? globalData.dynamicGlobalObject : globalObject);
 
         globalData.timeoutChecker.start();
-        JSValue retval;
-        if (handleEventFunction) {
-            retval = scriptExecutionContext->isDocument()
-                ? JSMainThreadExecState::call(exec, handleEventFunction, callType, callData, jsFunction, args)
-                : JSC::call(exec, handleEventFunction, callType, callData, jsFunction, args);
-        } else {
-            JSValue currentTarget = toJS(exec, globalObject, event->currentTarget());
-            retval = scriptExecutionContext->isDocument()
-                ? JSMainThreadExecState::call(exec, jsFunction, callType, callData, currentTarget, args)
-                : JSC::call(exec, jsFunction, callType, callData, currentTarget, args);
-        }
+        JSValue thisValue = handleEventFunction == jsFunction ? toJS(exec, globalObject, event->currentTarget()) : jsFunction;
+        JSValue retval = scriptExecutionContext->isDocument()
+            ? JSMainThreadExecState::call(exec, handleEventFunction, callType, callData, thisValue, args)
+            : JSC::call(exec, handleEventFunction, callType, callData, thisValue, args);
         globalData.timeoutChecker.stop();
 
         globalObject->setCurrentEvent(savedEvent);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to