Bug 1333838. Only treat actual boolean return values from OnErrorEventHandlerNonNull as being able to cancel the event. r=smaug
☠☠ backed out by bba40246e02c ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 26 Jan 2017 15:40:09 -0500
changeset 377950 9fae2a07dd437fd35c7952e8a01a6f40003b3c61
parent 377949 cc114ed3290c5f44eaf76e3730add4eb75234433
child 377951 f0488821fbbee3fcbf5e4567c3b0ce15a6c014e2
push id7198
push userjlorenzo@mozilla.com
push dateTue, 18 Apr 2017 12:07:49 +0000
treeherdermozilla-beta@d57aa49c3948 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1333838
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1333838. Only treat actual boolean return values from OnErrorEventHandlerNonNull as being able to cancel the event. r=smaug
dom/events/JSEventHandler.cpp
dom/webidl/EventHandler.webidl
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/html/webappapis/scripting/events/eventhandler-cancellation.html
--- a/dom/events/JSEventHandler.cpp
+++ b/dom/events/JSEventHandler.cpp
@@ -157,23 +157,24 @@ JSEventHandler::HandleEvent(nsIDOMEvent*
       scriptEvent->GetError(&error.Value());
     } else {
       msgOrEvent.SetAsEvent() = aEvent->InternalDOMEvent();
     }
 
     RefPtr<OnErrorEventHandlerNonNull> handler =
       mTypedHandler.OnErrorEventHandler();
     ErrorResult rv;
-    bool handled = handler->Call(mTarget, msgOrEvent, fileName, lineNumber,
-                                 columnNumber, error, rv);
+    JS::Rooted<JS::Value> retval(RootingCx());
+    handler->Call(mTarget, msgOrEvent, fileName, lineNumber,
+                  columnNumber, error, &retval, rv);
     if (rv.Failed()) {
       return rv.StealNSResult();
     }
 
-    if (handled) {
+    if (retval.isBoolean() && retval.toBoolean()) {
       event->PreventDefaultInternal(isChromeHandler);
     }
     return NS_OK;
   }
 
   if (mTypedHandler.Type() == TypedEventHandler::eOnBeforeUnload) {
     MOZ_ASSERT(mEventName == nsGkAtoms::onbeforeunload);
 
--- a/dom/webidl/EventHandler.webidl
+++ b/dom/webidl/EventHandler.webidl
@@ -10,23 +10,21 @@
  * Opera Software ASA. You are granted a license to use, reproduce
  * and create derivative works of this document.
  */
 [TreatNonObjectAsNull]
 callback EventHandlerNonNull = any (Event event);
 typedef EventHandlerNonNull? EventHandler;
 
 [TreatNonObjectAsNull]
-// https://www.w3.org/Bugs/Public/show_bug.cgi?id=23489
-//callback OnBeforeUnloadEventHandlerNonNull = DOMString (Event event);
 callback OnBeforeUnloadEventHandlerNonNull = DOMString? (Event event);
 typedef OnBeforeUnloadEventHandlerNonNull? OnBeforeUnloadEventHandler;
 
 [TreatNonObjectAsNull]
-callback OnErrorEventHandlerNonNull = boolean ((Event or DOMString) event, optional DOMString source, optional unsigned long lineno, optional unsigned long column, optional any error);
+callback OnErrorEventHandlerNonNull = any ((Event or DOMString) event, optional DOMString source, optional unsigned long lineno, optional unsigned long column, optional any error);
 typedef OnErrorEventHandlerNonNull? OnErrorEventHandler;
 
 [NoInterfaceObject]
 interface GlobalEventHandlers {
            attribute EventHandler onabort;
            attribute EventHandler onblur;
 // We think the spec is wrong here. See OnErrorEventHandlerForNodes/Window
 // below.
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -94696,16 +94696,22 @@
     ]
    ],
    "html/webappapis/scripting/events/event-handler-spec-example.html": [
     [
      "/html/webappapis/scripting/events/event-handler-spec-example.html",
      {}
     ]
    ],
+   "html/webappapis/scripting/events/eventhandler-cancellation.html": [
+    [
+     "/html/webappapis/scripting/events/eventhandler-cancellation.html",
+     {}
+    ]
+   ],
    "html/webappapis/scripting/events/inline-event-handler-ordering.html": [
     [
      "/html/webappapis/scripting/events/inline-event-handler-ordering.html",
      {}
     ]
    ],
    "html/webappapis/scripting/events/invalid-uncompiled-raw-handler-compiled-late.html": [
     [
@@ -175757,16 +175763,20 @@
   "html/webappapis/scripting/events/event-handler-processing-algorithm.html": [
    "33144e67a681a2f868628f7926efd60d5b078aba",
    "testharness"
   ],
   "html/webappapis/scripting/events/event-handler-spec-example.html": [
    "8d4d0bfdf447591695ac134cd243277ea2326c84",
    "testharness"
   ],
+  "html/webappapis/scripting/events/eventhandler-cancellation.html": [
+   "79b4a278f0e35646cfdffeebf8f0523e2772bc9b",
+   "testharness"
+  ],
   "html/webappapis/scripting/events/inline-event-handler-ordering.html": [
    "0bf73411e0a015a81aefc1327d4c7ef1ca824a56",
    "testharness"
   ],
   "html/webappapis/scripting/events/invalid-uncompiled-raw-handler-compiled-late.html": [
    "3a16be9d55c95a04eb426676ff89802974e88b95",
    "testharness"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/html/webappapis/scripting/events/eventhandler-cancellation.html
@@ -0,0 +1,76 @@
+<!doctype html>
+<meta charset=utf-8>
+<title></title>
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+<!-- A window to work with that won't trigger the harness exception detection
+     when we fire "error" events at it -->
+<iframe style="display: none"></iframe>
+<script>
+  test(function() {
+    var blob = new Blob([""]);
+    // Most targets disabled for now until
+    // https://github.com/whatwg/html/issues/2296 is sorted out.
+    var targets = [ frames[0] /*, document, document.documentElement,
+                    new Worker(URL.createObjectURL(blob) */ ];
+    // Event constructors also mostly disabled until
+    // https://github.com/whatwg/html/issues/2296 is sorted out.
+    var eventCtors = [ /* Event, */ ErrorEvent /*, MouseEvent */ ];
+    var values = [true, false, "", "abc", {}, 0, 1, -1, null, undefined,
+                  2.5, NaN, Infinity, Symbol.toStringTag ];
+    // Event types also mostly disabled pending
+    // https://github.com/whatwg/html/issues/2296
+    var eventTypes = [ "error"/*, "click", "load"*/ ];
+
+    // Variables that keep track of which subtest we're running.
+    var curTarget;
+    var curValue;
+    var curCtor;
+    var curType;
+
+    function defaultPreventedTester(event) {
+      var expectedValue;
+      if (curTarget === frames[0] &&
+          curCtor === ErrorEvent &&
+          curValue === true &&
+          curType == "error") {
+        expectedValue = true;
+      } else {
+        // This will need adjusting once we allow more targets and event
+        // constructors above!
+        expectedValue = false;
+      }
+      var valueRepr;
+      if (typeof curValue == "string") {
+        valueRepr = '"' + curValue + '"';
+      } else {
+        valueRepr = String(curValue);
+      }
+      test(function() {
+        assert_equals(event.defaultPrevented, expectedValue);
+      }, "Returning " + valueRepr +
+         " from " + String(curTarget) + "'s on" + curType +
+         " event handler while " + curCtor.name +
+         " is firing should" +
+         (expectedValue ? "" : " not") +
+         " cancel the event");
+    }
+
+    for (curCtor of eventCtors) {
+      for (curTarget of targets) {
+        for (curType of eventTypes) {
+          for (curValue of values) {
+            // We have to make sure that defaultPreventedTester is added after
+            // our event handler.
+            curTarget["on" + curType] = function() { return curValue; }
+            curTarget.addEventListener(curType, defaultPreventedTester);
+            var e = new curCtor(curType, { cancelable: true });
+            curTarget.dispatchEvent(e);
+            curTarget["on" + curType] = null;
+            curTarget.removeEventListener(curType, defaultPreventedTester);
+          }
+        }
+      }
+    }
+  }, "event handler cancellation behavior");
+</script>