- Revision
- 286873
- Author
- [email protected]
- Date
- 2021-12-10 13:28:43 -0800 (Fri, 10 Dec 2021)
Log Message
Extend the scope where the Window's current event is set
https://bugs.webkit.org/show_bug.cgi?id=233833
Reviewed by Ryosuke Niwa.
LayoutTests/imported/w3c:
Import WPT tests from https://github.com/web-platform-tests/wpt/pull/31894.
* web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result-expected.txt: Added.
* web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result.html: Added.
* web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any-expected.txt: Added.
* web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.html: Added.
* web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.js: Added.
* web-platform-tests/dom/events/resources/event-global-is-still-set-when-coercing-beforeunload-result-frame.html: Added.
Source/WebCore:
Inner invoke algorithm [1] sets window.event from step 8.2 until step 12 (inclusive).
That includes calling a callback interface [2], which performs "handleEvent" lookup
(step 10.1) and coerces return value of "beforeunload" handler (step 14).
Before this patch, window.event was not set during these user-observable operations.
Now WebKit is aligned with Blink and Gecko.
JSErrorHandler is correct: although reportException() may call userland "error" handler,
it will have window.event on its own.
[1] https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke
[2] https://webidl.spec.whatwg.org/#call-a-user-objects-operation
Tests: imported/w3c/web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result.html
imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.html
* bindings/js/JSEventListener.cpp:
(WebCore::JSEventListener::handleEvent):
ScopeExit is used since the method has so many exit points.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (286872 => 286873)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2021-12-10 21:25:01 UTC (rev 286872)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2021-12-10 21:28:43 UTC (rev 286873)
@@ -1,5 +1,21 @@
2021-12-10 Alexey Shvayka <[email protected]>
+ Extend the scope where the Window's current event is set
+ https://bugs.webkit.org/show_bug.cgi?id=233833
+
+ Reviewed by Ryosuke Niwa.
+
+ Import WPT tests from https://github.com/web-platform-tests/wpt/pull/31894.
+
+ * web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result-expected.txt: Added.
+ * web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result.html: Added.
+ * web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any-expected.txt: Added.
+ * web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.html: Added.
+ * web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.js: Added.
+ * web-platform-tests/dom/events/resources/event-global-is-still-set-when-coercing-beforeunload-result-frame.html: Added.
+
+2021-12-10 Alexey Shvayka <[email protected]>
+
JSErrorHandler should not set window.event if invocation target is in shadow tree
https://bugs.webkit.org/show_bug.cgi?id=233834
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result-expected.txt (0 => 286873)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result-expected.txt (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result-expected.txt 2021-12-10 21:28:43 UTC (rev 286873)
@@ -0,0 +1,4 @@
+
+
+PASS window.event is still set when 'beforeunload' result is coerced to string
+
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result.html (0 => 286873)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result.html (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result.html 2021-12-10 21:28:43 UTC (rev 286873)
@@ -0,0 +1,23 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>window.event is still set when 'beforeunload' result is coerced to string</title>
+<link rel="help" href=""
+<link rel="help" href=""
+
+<script src=""
+<script src=""
+
+<iframe id="iframe" src=""
+<body>
+<script>
+window._onload_ = () => {
+ async_test(t => {
+ iframe._onload_ = t.step_func_done(() => {
+ assert_equals(typeof window.currentEventInToString, "object");
+ assert_equals(window.currentEventInToString.type, "beforeunload");
+ });
+
+ iframe.contentWindow.location.href = ""
+ });
+};
+</script>
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any-expected.txt (0 => 286873)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any-expected.txt (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any-expected.txt 2021-12-10 21:28:43 UTC (rev 286873)
@@ -0,0 +1,3 @@
+
+PASS self.event is set before 'handleEvent' lookup
+
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.html (0 => 286873)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.html (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.html 2021-12-10 21:28:43 UTC (rev 286873)
@@ -0,0 +1 @@
+<!-- This file is required for WebKit test infrastructure to run the templated test -->
\ No newline at end of file
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.js (0 => 286873)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.js (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.js 2021-12-10 21:28:43 UTC (rev 286873)
@@ -0,0 +1,19 @@
+// https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke (steps 8.2 - 12)
+// https://webidl.spec.whatwg.org/#call-a-user-objects-operation (step 10.1)
+
+test(() => {
+ const eventTarget = new EventTarget;
+
+ let currentEvent;
+ eventTarget.addEventListener("foo", {
+ get handleEvent() {
+ currentEvent = self.event;
+ return () => {};
+ }
+ });
+
+ const event = new Event("foo");
+ eventTarget.dispatchEvent(event);
+
+ assert_equals(currentEvent, event);
+}, "self.event is set before 'handleEvent' lookup");
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/resources/event-global-is-still-set-when-coercing-beforeunload-result-frame.html (0 => 286873)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/resources/event-global-is-still-set-when-coercing-beforeunload-result-frame.html (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/resources/event-global-is-still-set-when-coercing-beforeunload-result-frame.html 2021-12-10 21:28:43 UTC (rev 286873)
@@ -0,0 +1,6 @@
+<!doctype html>
+<meta charset="utf-8">
+<body>
+<script>
+window._onbeforeunload_ = () => ({ toString: () => { parent.currentEventInToString = window.event; } });
+</script>
Modified: trunk/Source/WebCore/ChangeLog (286872 => 286873)
--- trunk/Source/WebCore/ChangeLog 2021-12-10 21:25:01 UTC (rev 286872)
+++ trunk/Source/WebCore/ChangeLog 2021-12-10 21:28:43 UTC (rev 286873)
@@ -1,5 +1,32 @@
2021-12-10 Alexey Shvayka <[email protected]>
+ Extend the scope where the Window's current event is set
+ https://bugs.webkit.org/show_bug.cgi?id=233833
+
+ Reviewed by Ryosuke Niwa.
+
+ Inner invoke algorithm [1] sets window.event from step 8.2 until step 12 (inclusive).
+ That includes calling a callback interface [2], which performs "handleEvent" lookup
+ (step 10.1) and coerces return value of "beforeunload" handler (step 14).
+
+ Before this patch, window.event was not set during these user-observable operations.
+ Now WebKit is aligned with Blink and Gecko.
+
+ JSErrorHandler is correct: although reportException() may call userland "error" handler,
+ it will have window.event on its own.
+
+ [1] https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke
+ [2] https://webidl.spec.whatwg.org/#call-a-user-objects-operation
+
+ Tests: imported/w3c/web-platform-tests/dom/events/event-global-is-still-set-when-coercing-beforeunload-result.html
+ imported/w3c/web-platform-tests/dom/events/event-global-set-before-handleEvent-lookup.any.html
+
+ * bindings/js/JSEventListener.cpp:
+ (WebCore::JSEventListener::handleEvent):
+ ScopeExit is used since the method has so many exit points.
+
+2021-12-10 Alexey Shvayka <[email protected]>
+
JSErrorHandler should not set window.event if invocation target is in shadow tree
https://bugs.webkit.org/show_bug.cgi?id=233834
Modified: trunk/Source/WebCore/bindings/js/JSEventListener.cpp (286872 => 286873)
--- trunk/Source/WebCore/bindings/js/JSEventListener.cpp 2021-12-10 21:25:01 UTC (rev 286872)
+++ trunk/Source/WebCore/bindings/js/JSEventListener.cpp 2021-12-10 21:28:43 UTC (rev 286873)
@@ -41,6 +41,7 @@
#include <_javascript_Core/VMEntryScope.h>
#include <_javascript_Core/Watchdog.h>
#include <wtf/Ref.h>
+#include <wtf/Scope.h>
namespace WebCore {
using namespace JSC;
@@ -137,6 +138,21 @@
return;
}
+ RefPtr<Event> savedEvent;
+ auto* jsFunctionWindow = jsDynamicCast<JSDOMWindow*>(vm, jsFunction->globalObject(vm));
+ if (jsFunctionWindow) {
+ savedEvent = jsFunctionWindow->currentEvent();
+
+ // window.event should not be set when the target is inside a shadow tree, as per the DOM specification.
+ if (!event.currentTargetIsInShadowTree())
+ jsFunctionWindow->setCurrentEvent(&event);
+ }
+
+ auto restoreCurrentEventOnExit = makeScopeExit([&] {
+ if (jsFunctionWindow)
+ jsFunctionWindow->setCurrentEvent(savedEvent.get());
+ });
+
JSGlobalObject* lexicalGlobalObject = jsFunction->globalObject();
JSValue handleEventFunction = jsFunction;
@@ -170,16 +186,6 @@
args.append(toJS(lexicalGlobalObject, globalObject, &event));
ASSERT(!args.hasOverflowed());
- RefPtr<Event> savedEvent;
- auto* jsFunctionWindow = jsDynamicCast<JSDOMWindow*>(vm, jsFunction->globalObject());
- if (jsFunctionWindow) {
- savedEvent = jsFunctionWindow->currentEvent();
-
- // window.event should not be set when the target is inside a shadow tree, as per the DOM specification.
- if (!event.currentTargetIsInShadowTree())
- jsFunctionWindow->setCurrentEvent(&event);
- }
-
VMEntryScope entryScope(vm, vm.entryScope ? vm.entryScope->globalObject() : lexicalGlobalObject);
JSExecState::instrumentFunction(&scriptExecutionContext, callData);
@@ -190,9 +196,6 @@
InspectorInstrumentation::didCallFunction(&scriptExecutionContext);
- if (jsFunctionWindow)
- jsFunctionWindow->setCurrentEvent(savedEvent.get());
-
auto handleExceptionIfNeeded = [&] (JSC::Exception* exception) -> bool {
if (is<WorkerGlobalScope>(scriptExecutionContext)) {
auto* scriptController = downcast<WorkerGlobalScope>(scriptExecutionContext).script();
