LibWeb: Check if event is not "beforeunload" before cancelling

Author: https://github.com/FMMazur
Commit: https://github.com/LadybirdBrowser/ladybird/commit/f6c4304e893
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/3320
Reviewed-by: https://github.com/tcl3 ✅

Libraries/LibWeb/DOM/EventTarget.cpp

@@ -2,6 +2,7 @@
  * Copyright (c) 2020-2022, Andreas Kling <andreas@ladybird.org>
  * Copyright (c) 2022, Luke Wilde <lukew@serenityos.org>
  * Copyright (c) 2024, Glenn Skrzypczak <glenn.skrzypczak@gmail.com>
+ * Copyright (c) 2025, Felipe Muñoz Mazur <felipe.munoz.mazur@protonmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -716,8 +717,10 @@ JS::ThrowCompletionOr<void> EventTarget::process_event_handler_for_event(FlyStri
     // FIXME: Ideally, invoke_callback would convert JS::Value to the appropriate return type for us as per the spec, but it doesn't currently.
     auto return_value = *return_value_or_error.value();
 
+    auto is_beforeunload = event.type() == HTML::EventNames::beforeunload;
+
     // 5. Process return value as follows:
-    if (is<HTML::BeforeUnloadEvent>(event) && event.type() == "beforeunload") {
+    if (is<HTML::BeforeUnloadEvent>(event) && is_beforeunload) {
         // ->  If event is a BeforeUnloadEvent object and event's type is "beforeunload"
         //         If return value is not null, then:
         if (!return_value.is_nullish()) {
@@ -741,7 +744,10 @@ JS::ThrowCompletionOr<void> EventTarget::process_event_handler_for_event(FlyStri
         // -> Otherwise
         //      If return value is false, then set event's canceled flag.
         // NOTE: the return type of EventHandler is `any`, so no coercion happens, meaning we have to check if it's a boolean first.
-        if (return_value.is_boolean() && !return_value.as_bool())
+        // Spec-Note: If we've gotten to this "Otherwise" clause because event's type is "beforeunload" but event is
+        //            not a BeforeUnloadEvent object, then return value will never be false, since in such cases
+        //            return value will have been coerced into either null or a DOMString.
+        if (return_value.is_boolean() && !return_value.as_bool() && !is_beforeunload)
             event.set_cancelled(true);
     }
 

Tests/LibWeb/Text/expected/wpt-import/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling.txt

@@ -0,0 +1,27 @@
+Harness status: OK
+
+Found 22 tests
+
+22 Pass
+Pass	Returning a string must not cancel the event: CustomEvent, non-cancelable
+Pass	Returning a string must not cancel the event: CustomEvent, cancelable
+Pass	Returning false must not cancel the event, because it's coerced to the DOMString "false" which does not cancel CustomEvents: CustomEvent, cancelable
+Pass	Returning a string must not cancel the event: BeforeUnloadEvent with type "click", cancelable
+Pass	Returning null with a real iframe unloading
+Pass	Returning undefined with a real iframe unloading
+Pass	Returning  with a real iframe unloading
+Pass	Returning false with a real iframe unloading
+Pass	Returning true with a real iframe unloading
+Pass	Returning 0 with a real iframe unloading
+Pass	Returning null with a real iframe unloading; setting returnValue to foo
+Pass	Returning undefined with a real iframe unloading; setting returnValue to foo
+Pass	Returning  with a real iframe unloading; setting returnValue to foo
+Pass	Returning false with a real iframe unloading; setting returnValue to foo
+Pass	Returning true with a real iframe unloading; setting returnValue to foo
+Pass	Returning 0 with a real iframe unloading; setting returnValue to foo
+Pass	Returning undefined with a real iframe unloading; setting returnValue to 
+Pass	Returning undefined with a real iframe unloading; calling preventDefault()
+Pass	Returning undefined with a real iframe unloading; setting returnValue to foo; calling preventDefault()
+Pass	Returning foo with a real iframe unloading; calling preventDefault()
+Pass	Returning foo with a real iframe unloading; setting returnValue to foo; calling preventDefault()
+Pass	Returning true with a real iframe unloading; setting returnValue to ; calling preventDefault()

Tests/LibWeb/Text/input/wpt-import/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling.html

@@ -0,0 +1,222 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>beforeunload return value cancelation behavior</title>
+<link rel="help" href="https://html.spec.whatwg.org/multipage/webappapis.html#the-event-handler-processing-algorithm">
+<link rel="author" title="Domenic Denicola" href="mailto:d@domenic.me">
+<script src="../../../../resources/testharness.js"></script>
+<script src="../../../../resources/testharnessreport.js"></script>
+
+<div id="log"></div>
+
+<script>
+"use strict";
+
+promise_test(t => {
+  let onbeforeunloadHappened = false;
+  window.onbeforeunload = t.step_func(() => {
+    onbeforeunloadHappened = true;
+    return "cancel me";
+  });
+
+  const eventWatcher = new EventWatcher(t, window, "beforeunload");
+  const promise = eventWatcher.wait_for("beforeunload").then(e => {
+    assert_true(onbeforeunloadHappened, "CustomEvent must be able to trigger the event handler");
+    assert_false(e.defaultPrevented, "The event must not have been canceled");
+    window.onbeforeunload = null;
+  });
+
+  window.dispatchEvent(new CustomEvent("beforeunload"));
+
+  return promise;
+}, "Returning a string must not cancel the event: CustomEvent, non-cancelable");
+
+promise_test(t => {
+  let onbeforeunloadHappened = false;
+  window.onbeforeunload = t.step_func(() => {
+    onbeforeunloadHappened = true;
+    return "cancel me";
+  });
+
+  const eventWatcher = new EventWatcher(t, window, "beforeunload");
+  const promise = eventWatcher.wait_for("beforeunload").then(e => {
+    assert_true(onbeforeunloadHappened, "CustomEvent must be able to trigger the event handler");
+    assert_false(e.defaultPrevented, "The event must not have been canceled");
+    window.onbeforeunload = null;
+    t.done();
+  });
+
+  window.dispatchEvent(new CustomEvent("beforeunload", { cancelable: true }));
+
+  return promise;
+}, "Returning a string must not cancel the event: CustomEvent, cancelable");
+
+promise_test(t => {
+  let onbeforeunloadHappened = false;
+  window.onbeforeunload = t.step_func(() => {
+    onbeforeunloadHappened = true;
+    return false;
+  });
+
+  const eventWatcher = new EventWatcher(t, window, "beforeunload");
+  const promise = eventWatcher.wait_for("beforeunload").then(e => {
+    assert_true(onbeforeunloadHappened, "CustomEvent must be able to trigger the event handler");
+    assert_false(e.defaultPrevented, "The event must not have been canceled");
+    window.onbeforeunload = null;
+    t.done();
+  });
+
+  window.dispatchEvent(new CustomEvent("beforeunload", { cancelable: true }));
+
+  return promise;
+}, "Returning false must not cancel the event, because it's coerced to the DOMString \"false\" which does not cancel " +
+   "CustomEvents: CustomEvent, cancelable");
+
+// This test can be removed if we update the DOM Standard to disallow createEvent("BeforeUnloadEvent"). Browser support
+// is inconsistent. https://github.com/whatwg/dom/issues/362
+promise_test(t => {
+  const eventWatcher = new EventWatcher(t, window, "click");
+  const promise = eventWatcher.wait_for("click").then(e => {
+    assert_false(e.defaultPrevented, "The event must not have been canceled");
+    window.onbeforeunload = null;
+    t.done();
+  });
+
+  const ev = document.createEvent("BeforeUnloadEvent");
+  ev.initEvent("click", false, true);
+  window.dispatchEvent(ev);
+
+  return promise;
+}, "Returning a string must not cancel the event: BeforeUnloadEvent with type \"click\", cancelable");
+
+const testCases = [
+  {
+    valueToReturn: null,
+    expectCancelation: false,
+    expectedReturnValue: ""
+  },
+  {
+    valueToReturn: undefined,
+    expectCancelation: false,
+    expectedReturnValue: ""
+  },
+  {
+    valueToReturn: "",
+    expectCancelation: true,
+    expectedReturnValue: ""
+  },
+  {
+    valueToReturn: false,
+    expectCancelation: true,
+    expectedReturnValue: "false"
+  },
+  {
+    valueToReturn: true,
+    expectCancelation: true,
+    expectedReturnValue: "true"
+  },
+  {
+    valueToReturn: 0,
+    expectCancelation: true,
+    expectedReturnValue: "0"
+  },
+  {
+    valueToReturn: null,
+    expectCancelation: false,
+    setReturnValue: "foo",
+    expectedReturnValue: "foo"
+  },
+  {
+    valueToReturn: undefined,
+    expectCancelation: false,
+    setReturnValue: "foo",
+    expectedReturnValue: "foo"
+  },
+  {
+    valueToReturn: "",
+    expectCancelation: true,
+    setReturnValue: "foo",
+    expectedReturnValue: "foo"
+  },
+  {
+    valueToReturn: false,
+    expectCancelation: true,
+    setReturnValue: "foo",
+    expectedReturnValue: "foo"
+  },
+  {
+    valueToReturn: true,
+    expectCancelation: true,
+    setReturnValue: "foo",
+    expectedReturnValue: "foo"
+  },
+  {
+    valueToReturn: 0,
+    expectCancelation: true,
+    setReturnValue: "foo",
+    expectedReturnValue: "foo"
+  },
+  {
+    setReturnValue: "",
+    expectedReturnValue: "",
+    expectCancelation: false,
+  },
+  {
+    expectCancelation: true,
+    expectedReturnValue: "",
+    cancel: true
+  },
+  {
+    setReturnValue: "foo",
+    expectCancelation: true,
+    expectedReturnValue: "foo",
+    cancel: true
+  },
+  {
+    valueToReturn: "foo",
+    expectedReturnValue: "foo",
+    expectCancelation: true,
+    cancel: true
+  },
+  {
+    valueToReturn: "foo",
+    setReturnValue: "foo",
+    expectedReturnValue: "foo",
+    expectCancelation: true,
+    cancel: true
+  },
+  {
+    valueToReturn: true,
+    setReturnValue: "",
+    expectedReturnValue: "true",
+    expectCancelation: true,
+    cancel: true
+  }
+];
+
+var testCaseIndex = 0;
+function runNextTest() {
+  const testCase = testCases[testCaseIndex];
+
+  const labelAboutReturnValue = testCase.setReturnValue === undefined ? "" :
+    `; setting returnValue to ${testCase.setReturnValue}`;
+
+  const labelAboutCancel = testCase.cancel === undefined ? "" :
+    "; calling preventDefault()";
+
+  const suffixLabels = labelAboutReturnValue + labelAboutCancel;
+
+  async_test(t => {
+    const iframe = document.createElement("iframe");
+    iframe.onload = t.step_func(() => {
+      iframe.contentWindow.runTest(t, testCase);
+      if (++testCaseIndex < testCases.length)
+        runNextTest();
+    });
+
+    iframe.src = "support/beforeunload-canceling-1.html";
+    document.body.appendChild(iframe);
+  }, `Returning ${testCase.valueToReturn} with a real iframe unloading${suffixLabels}`);
+}
+
+runNextTest();
+</script>

Tests/LibWeb/Text/input/wpt-import/html/browsers/browsing-the-web/unloading-documents/support/beforeunload-canceling-1.html

@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Support page for beforeunload-canceling.html</title>
+
+<h1>If this goes away, it navigated</h1>
+
+<script>
+"use strict";
+
+window.runTest = (t, { valueToReturn, expectCancelation, setReturnValue, expectedReturnValue, cancel }) => {
+  window.onbeforeunload = t.step_func(e => {
+    if (cancel) {
+      e.preventDefault();
+    }
+
+    if (setReturnValue !== undefined) {
+      e.returnValue = setReturnValue;
+    }
+
+    return valueToReturn;
+  });
+
+  const listener = t.step_func(e => {
+    top.assert_equals(e.defaultPrevented, expectCancelation, "canceled");
+    top.assert_equals(e.returnValue, expectedReturnValue, "returnValue");
+    window.onbeforeunload = null;
+
+    t.done();
+  });
+
+  window.addEventListener("beforeunload", listener);
+
+  window.location.href = "about:blank";
+};
+</script>