Bug 1345996. Change event handler invocation to only do the "true ret…

…urn cancels" for onerror handlers handed ErrorEvents, and only on globals. r=smaug See whatwg/html#2296 and whatwg/html#423 for details on what various browsers do and whatnot. MozReview-Commit-ID: DytkZreHudx
Manishearth · Mar 11, 2017 · 39151c0 · 39151c0
1 parent a6f9813
commit 39151c0
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 27 deletions.
diff --git a/dom/events/JSEventHandler.cpp b/dom/events/JSEventHandler.cpp
@@ -169,7 +169,8 @@ JSEventHandler::HandleEvent(nsIDOMEvent* aEvent)
       return rv.StealNSResult();
     }
 
-    if (retval.isBoolean() && retval.toBoolean()) {
+    if (retval.isBoolean() &&
+        retval.toBoolean() == bool(scriptEvent)) {
       event->PreventDefaultInternal(isChromeHandler);
     }
     return NS_OK;
@@ -216,12 +217,8 @@ JSEventHandler::HandleEvent(nsIDOMEvent* aEvent)
     return rv.StealNSResult();
   }
 
-  // If the handler returned false and its sense is not reversed,
-  // or the handler returned true and its sense is reversed from
-  // the usual (false means cancel), then prevent default.
-  if (retval.isBoolean() &&
-      retval.toBoolean() == (mEventName == nsGkAtoms::onerror ||
-                             mEventName == nsGkAtoms::onmouseover)) {
+  // If the handler returned false, then prevent default.
+  if (retval.isBoolean() && !retval.toBoolean()) {
     event->PreventDefaultInternal(isChromeHandler);
   }
 

diff --git a/...latform/meta/html/webappapis/scripting/events/event-handler-processing-algorithm.html.ini b/...latform/meta/html/webappapis/scripting/events/event-handler-processing-algorithm.html.ini
diff --git a/...b-platform/tests/html/webappapis/scripting/events/event-handler-processing-algorithm.html b/...b-platform/tests/html/webappapis/scripting/events/event-handler-processing-algorithm.html
@@ -5,45 +5,56 @@
  <body>
  <div id="foo" style="width: 100px; height: 100px; background-color: black"></div>
  <script>
- async_test(function(t) {
+
+ // Historically mouseover was special in the spec, but now it is not. See https://github.com/whatwg/html/pull/2398.
+ test(function(t) {
      var ev = new Event('mouseover', {cancelable: true});
-     document.getElementById("foo").onmouseover = t.step_func(function() { return true });
+     document.getElementById("foo").onmouseover = t.step_func(function() { return false });
+     document.getElementById("foo").dispatchEvent(ev);
+     assert_equals(ev.defaultPrevented, true)
+ }, "mouseover listener returning false cancels event (using Event)");
+
+ test(function(t) {
+     var ev = new MouseEvent('mouseover', {cancelable: true});
+     document.getElementById("foo").onmouseover = t.step_func(function() { return false });
      document.getElementById("foo").dispatchEvent(ev);
      assert_equals(ev.defaultPrevented, true)
-     t.done();
- }, "mouseover listener returning true cancels event");
+ }, "mouseover listener returning false cancels event (using MouseEvent)");
 
- async_test(function(t) {
+ test(function(t) {
      var ev = new Event('mouseover', {cancelable: true});
-     document.getElementById("foo").onmouseover = t.step_func(function() { return false; });
+     document.getElementById("foo").onmouseover = t.step_func(function() { return true });
+     document.getElementById("foo").dispatchEvent(ev);
+     assert_equals(ev.defaultPrevented, false)
+ }, "mouseover listener returning true doesn't cancel event (using Event)");
+
+ test(function(t) {
+     var ev = new MouseEvent('mouseover', {cancelable: true});
+     document.getElementById("foo").onmouseover = t.step_func(function() { return true });
      document.getElementById("foo").dispatchEvent(ev);
-     assert_equals(ev.defaultPrevented, false);
-     t.done();
- }, "mouseover listener returning false doesn't cancel event");
+     assert_equals(ev.defaultPrevented, false)
+ }, "mouseover listener returning true doesn't cancel event (using MouseEvent)");
 
  // beforeunload is tested in html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling.html
 
- async_test(function(t) {
+ test(function(t) {
      var ev = new Event("click", {cancelable: true});
      document.getElementById("foo").onclick = t.step_func(function() { return false; });
      document.getElementById("foo").dispatchEvent(ev);
      assert_equals(ev.defaultPrevented, true);
-     t.done();
  }, "click listener returning false cancels event");
 
- async_test(function(t) {
+ test(function(t) {
      var ev = new Event("blur", {cancelable: true});
      document.getElementById("foo").onblur = t.step_func(function() { return false; });
      document.getElementById("foo").dispatchEvent(ev);
      assert_equals(ev.defaultPrevented, true);
-     t.done();
  }, "blur listener returning false cancels event");
 
- async_test(function(t) {
+ test(function(t) {
      var ev = new Event("dblclick", {cancelable: true});
      document.getElementById("foo").ondblclick = t.step_func(function() { return false; });
      document.getElementById("foo").dispatchEvent(ev);
      assert_equals(ev.defaultPrevented, true);
-     t.done();
  }, "dblclick listener returning false cancels event");
 </script>