Modified: trunk/LayoutTests/ChangeLog (236632 => 236633)
--- trunk/LayoutTests/ChangeLog 2018-09-29 01:53:34 UTC (rev 236632)
+++ trunk/LayoutTests/ChangeLog 2018-09-29 02:03:51 UTC (rev 236633)
@@ -1,3 +1,17 @@
+2018-09-28 Chris Dumez <[email protected]>
+
+ The return value of an OnBeforeUnloadEventHandler should always be coerced into a DOMString
+ https://bugs.webkit.org/show_bug.cgi?id=190090
+
+ Reviewed by Ryosuke Niwa.
+
+ Update test that was returning a value in a beforeunload event listener instead of using an
+ event handler. The test needs to use an event handler (window.onbeforeunload) as an event
+ listener does not have a return value. I have verified that our behavior is consistent with
+ Chrome and Firefox on this test, both with an event listener and an event handler.
+
+ * fast/loader/form-submission-after-beforeunload-cancel.html:
+
2018-09-28 Simon Fraser <[email protected]>
RenderLayer::removeOnlyThisLayer() should not call updateLayerPositions()
Modified: trunk/LayoutTests/fast/loader/form-submission-after-beforeunload-cancel.html (236632 => 236633)
--- trunk/LayoutTests/fast/loader/form-submission-after-beforeunload-cancel.html 2018-09-29 01:53:34 UTC (rev 236632)
+++ trunk/LayoutTests/fast/loader/form-submission-after-beforeunload-cancel.html 2018-09-29 02:03:51 UTC (rev 236633)
@@ -13,7 +13,7 @@
document.forms[0].submit();
}
-window.addEventListener("beforeunload", function() {
+_onbeforeunload_ = function() {
if (window._confirmationDialogDisplayedOnce)
return "Click 'Leave Page'";
@@ -34,7 +34,7 @@
window._confirmationDialogDisplayedOnce = true;
return "Click 'Stay on Page'";
-});
+}
</script>
<p>This tests that submitting a form a second time after canceling the first submission in a onbeforeunload handler is allowed. To test manually, follow the instructions in the _javascript_ confirmation dialogs.</p>
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (236632 => 236633)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2018-09-29 01:53:34 UTC (rev 236632)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2018-09-29 02:03:51 UTC (rev 236633)
@@ -1,5 +1,16 @@
2018-09-28 Chris Dumez <[email protected]>
+ The return value of an OnBeforeUnloadEventHandler should always be coerced into a DOMString
+ https://bugs.webkit.org/show_bug.cgi?id=190090
+
+ Reviewed by Ryosuke Niwa.
+
+ Rebaseline WPT test now that one more check is passing.
+
+ * web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt:
+
+2018-09-28 Chris Dumez <[email protected]>
+
document.open() should throw errors for cross-origin calls
https://bugs.webkit.org/show_bug.cgi?id=189371
<rdar://problem/44282700>
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt (236632 => 236633)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt 2018-09-29 01:53:34 UTC (rev 236632)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt 2018-09-29 02:03:51 UTC (rev 236633)
@@ -1,7 +1,7 @@
PASS Returning a string must not cancel the event: CustomEvent, non-cancelable
PASS Returning a string must not cancel the event: CustomEvent, cancelable
-FAIL Returning false must not cancel the event, because it's coerced to the DOMString "false" which does not cancel CustomEvents: CustomEvent, cancelable assert_false: The event must not have been canceled expected false got true
+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
Modified: trunk/Source/WebCore/ChangeLog (236632 => 236633)
--- trunk/Source/WebCore/ChangeLog 2018-09-29 01:53:34 UTC (rev 236632)
+++ trunk/Source/WebCore/ChangeLog 2018-09-29 02:03:51 UTC (rev 236633)
@@ -1,3 +1,24 @@
+2018-09-28 Chris Dumez <[email protected]>
+
+ The return value of an OnBeforeUnloadEventHandler should always be coerced into a DOMString
+ https://bugs.webkit.org/show_bug.cgi?id=190090
+
+ Reviewed by Ryosuke Niwa.
+
+ The return value of an OnBeforeUnloadEventHandler should always be coerced into a DOMString:
+ - https://html.spec.whatwg.org/#onbeforeunloadeventhandler
+ - https://html.spec.whatwg.org/#the-event-handler-processing-algorithm (Step 5)
+
+ In particular, this means that returning false in an OnBeforeUnloadEventHandler should NOT
+ cancel the event when the event is a CustomEvent (and not a BeforeUnloadEvent). This is
+ because the return value cannot be false at:
+ - https://html.spec.whatwg.org/#the-event-handler-processing-algorithm (Step 5. Otherwise case).
+
+ No new tests, rebaselined existing test.
+
+ * bindings/js/JSEventListener.cpp:
+ (WebCore::JSEventListener::handleEvent):
+
2018-09-28 Simon Fraser <[email protected]>
RenderLayer::removeOnlyThisLayer() should not call updateLayerPositions()
Modified: trunk/Source/WebCore/bindings/js/JSEventListener.cpp (236632 => 236633)
--- trunk/Source/WebCore/bindings/js/JSEventListener.cpp 2018-09-29 01:53:34 UTC (rev 236632)
+++ trunk/Source/WebCore/bindings/js/JSEventListener.cpp 2018-09-29 02:03:51 UTC (rev 236633)
@@ -149,52 +149,63 @@
callType = getCallData(vm, handleEventFunction, callData);
}
- if (callType != CallType::None) {
- Ref<JSEventListener> protectedThis(*this);
+ if (callType == CallType::None)
+ return;
- MarkedArgumentBuffer args;
- args.append(toJS(exec, globalObject, &event));
- ASSERT(!args.hasOverflowed());
+ Ref<JSEventListener> protectedThis(*this);
- Event* savedEvent = globalObject->currentEvent();
+ MarkedArgumentBuffer args;
+ args.append(toJS(exec, globalObject, &event));
+ ASSERT(!args.hasOverflowed());
- // window.event should not be set when the target is inside a shadow tree, as per the DOM specification.
- bool isTargetInsideShadowTree = is<Node>(event.currentTarget()) && downcast<Node>(*event.currentTarget()).isInShadowTree();
- if (!isTargetInsideShadowTree)
- globalObject->setCurrentEvent(&event);
+ Event* savedEvent = globalObject->currentEvent();
- VMEntryScope entryScope(vm, vm.entryScope ? vm.entryScope->globalObject() : globalObject);
+ // window.event should not be set when the target is inside a shadow tree, as per the DOM specification.
+ bool isTargetInsideShadowTree = is<Node>(event.currentTarget()) && downcast<Node>(*event.currentTarget()).isInShadowTree();
+ if (!isTargetInsideShadowTree)
+ globalObject->setCurrentEvent(&event);
- InspectorInstrumentationCookie cookie = JSExecState::instrumentFunctionCall(&scriptExecutionContext, callType, callData);
+ VMEntryScope entryScope(vm, vm.entryScope ? vm.entryScope->globalObject() : globalObject);
- JSValue thisValue = handleEventFunction == jsFunction ? toJS(exec, globalObject, event.currentTarget()) : jsFunction;
- NakedPtr<JSC::Exception> exception;
- JSValue retval = JSExecState::profiledCall(exec, JSC::ProfilingReason::Other, handleEventFunction, callType, callData, thisValue, args, exception);
+ InspectorInstrumentationCookie cookie = JSExecState::instrumentFunctionCall(&scriptExecutionContext, callType, callData);
- InspectorInstrumentation::didCallFunction(cookie, &scriptExecutionContext);
+ JSValue thisValue = handleEventFunction == jsFunction ? toJS(exec, globalObject, event.currentTarget()) : jsFunction;
+ NakedPtr<JSC::Exception> exception;
+ JSValue retval = JSExecState::profiledCall(exec, JSC::ProfilingReason::Other, handleEventFunction, callType, callData, thisValue, args, exception);
- globalObject->setCurrentEvent(savedEvent);
+ InspectorInstrumentation::didCallFunction(cookie, &scriptExecutionContext);
- if (is<WorkerGlobalScope>(scriptExecutionContext)) {
- auto& scriptController = *downcast<WorkerGlobalScope>(scriptExecutionContext).script();
- bool terminatorCausedException = (scope.exception() && isTerminatedExecutionException(vm, scope.exception()));
- if (terminatorCausedException || scriptController.isTerminatingExecution())
- scriptController.forbidExecution();
- }
+ globalObject->setCurrentEvent(savedEvent);
- if (exception) {
- event.target()->uncaughtExceptionInEventHandler();
- reportException(exec, exception);
- } else {
- if (is<BeforeUnloadEvent>(event))
- handleBeforeUnloadEventReturnValue(downcast<BeforeUnloadEvent>(event), convert<IDLNullable<IDLDOMString>>(*exec, retval));
+ if (is<WorkerGlobalScope>(scriptExecutionContext)) {
+ auto& scriptController = *downcast<WorkerGlobalScope>(scriptExecutionContext).script();
+ bool terminatorCausedException = (scope.exception() && isTerminatedExecutionException(vm, scope.exception()));
+ if (terminatorCausedException || scriptController.isTerminatingExecution())
+ scriptController.forbidExecution();
+ }
- if (m_isAttribute) {
- if (retval.isFalse())
- event.preventDefault();
- }
- }
+ if (exception) {
+ event.target()->uncaughtExceptionInEventHandler();
+ reportException(exec, exception);
+ return;
}
+
+ if (!m_isAttribute) {
+ // This is an EventListener and there is therefore no need for any return value handling.
+ return;
+ }
+
+ // Do return value handling for event handlers (https://html.spec.whatwg.org/#the-event-handler-processing-algorithm).
+
+ if (event.type() == eventNames().beforeunloadEvent) {
+ // This is a OnBeforeUnloadEventHandler, and therefore the return value must be coerced into a String.
+ if (is<BeforeUnloadEvent>(event))
+ handleBeforeUnloadEventReturnValue(downcast<BeforeUnloadEvent>(event), convert<IDLNullable<IDLDOMString>>(*exec, retval));
+ return;
+ }
+
+ if (retval.isFalse())
+ event.preventDefault();
}
bool JSEventListener::operator==(const EventListener& listener) const
