Changeset 269983 in webkit

Timestamp:

Nov 18, 2020 1:57:01 PM (3 years ago)

Author:

Chris Dumez

Message:

[iOS] beforeunload event does not fire in MobileSafari
​https://bugs.webkit.org/show_bug.cgi?id=219102
<rdar://problem/70550655>

Reviewed by Geoff Garen.

Source/WebCore:

MobileSafari on iOS does not implement WKUIDelegate's runJavaScriptAlertPanelWithMessage because
it never shows any before unload prompt. When the client does not implement this delegate,
Chrome::canRunBeforeUnloadConfirmPanel() returns false and this was causing
FrameLoader::shouldClose() to return early, before even firing the beforeunload event in each
frame. I updated our logic so that we now fire the beforeunload events no matter what and we
merely do not attempt to show the beforeunload prompt when Chrome::canRunBeforeUnloadConfirmPanel()
return false, similarly to what we do when the document does not have a user gesture.

Note that we already fire the pagehide and unload events on iOS so this is not a significant
change in policy.

• loader/FrameLoader.cpp:

(WebCore::FrameLoader::shouldClose):
(WebCore::FrameLoader::dispatchBeforeUnloadEvent):

Tools:

Add API test coverage.

• TestWebKitAPI/Tests/WebKit/beforeunload.html:
• TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm:

(-[UIDelegateWithoutRunBeforeUnload webViewDidClose:]):
(-[BeforeUnloadMessageHandler userContentController:didReceiveScriptMessage:]):
(TEST):

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/FrameLoader.cpp (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit/beforeunload.html (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-11-18  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] beforeunload event does not fire in MobileSafari
+        https://bugs.webkit.org/show_bug.cgi?id=219102
+        <rdar://problem/70550655>
+
+        Reviewed by Geoff Garen.
+
+        MobileSafari on iOS does not implement WKUIDelegate's runJavaScriptAlertPanelWithMessage because
+        it never shows any before unload prompt. When the client does not implement this delegate,
+        Chrome::canRunBeforeUnloadConfirmPanel() returns false and this was causing
+        FrameLoader::shouldClose() to return early, before even firing the beforeunload event in each
+        frame. I updated our logic so that we now fire the beforeunload events no matter what and we
+        merely do not attempt to show the beforeunload prompt when Chrome::canRunBeforeUnloadConfirmPanel()
+        return false, similarly to what we do when the document does not have a user gesture.
+
+        Note that we already fire the pagehide and unload events on iOS so this is not a significant
+        change in policy.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::shouldClose):
+        (WebCore::FrameLoader::dispatchBeforeUnloadEvent):
+
 2020-11-18  Aditya Keerthi  <akeerthi@apple.com>
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -3235 +3235 @@
     if (!page)
         return true;
-    if (!page->chrome().canRunBeforeUnloadConfirmPanel())
-        return true;
 
     // Store all references to each subframe in advance since beforeunload's event handler may modify frame
@@ -3366 +3364 @@
         document->defaultEventHandler(beforeUnloadEvent.get());
 
-    if (!shouldAskForNavigationConfirmation(*document, beforeUnloadEvent))
+    if (!chrome.canRunBeforeUnloadConfirmPanel() || !shouldAskForNavigationConfirmation(*document, beforeUnloadEvent))
         return true;
 

trunk/Tools/ChangeLog

@@ +1 @@
+2020-11-18  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] beforeunload event does not fire in MobileSafari
+        https://bugs.webkit.org/show_bug.cgi?id=219102
+        <rdar://problem/70550655>
+
+        Reviewed by Geoff Garen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKit/beforeunload.html:
+        * TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm:
+        (-[UIDelegateWithoutRunBeforeUnload webViewDidClose:]):
+        (-[BeforeUnloadMessageHandler userContentController:didReceiveScriptMessage:]):
+        (TEST):
+
 2020-11-18  Chris Dumez  <cdumez@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKit/beforeunload.html

@@ -4 +4 @@
 <script>
 onbeforeunload = (ev) => {
+    if (window.webkit && webkit.messageHandlers.testHandler)
+        webkit.messageHandlers.testHandler.postMessage("beforeunload called");
     ev.preventDefault();
     return "foo";

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm

@@ -314 +314 @@
 
 #endif
+
+static bool wasBeforeUnloadEventHandlerCalled = false;
+
+@interface UIDelegateWithoutRunBeforeUnload : NSObject <WKUIDelegate>
+@end
+
+@implementation UIDelegateWithoutRunBeforeUnload
+
+- (void)webViewDidClose:(WKWebView *)webView
+{
+    [webView _close];
+}
+
+@end
+
+@interface BeforeUnloadMessageHandler : NSObject <WKScriptMessageHandler>
+@end
+
+@implementation BeforeUnloadMessageHandler
+- (void)userContentController:(WKUserContentController *)userContentController didReceiveScriptMessage:(WKScriptMessage *)message
+{
+    NSString *scriptMessage = [message body];
+    EXPECT_WK_STREQ(@"beforeunload called", scriptMessage);
+    wasBeforeUnloadEventHandlerCalled = true;
+}
+@end
+
+TEST(WebKit, BeforeUnloadEventWithoutRunBeforeUnloadConfirmPanelUIDelegate)
+{
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    auto messageHandler = adoptNS([[BeforeUnloadMessageHandler alloc] init]);
+    [[webViewConfiguration userContentController] addScriptMessageHandler:messageHandler.get() name:@"testHandler"];
+
+    auto uiDelegate = adoptNS([[UIDelegateWithoutRunBeforeUnload alloc] init]);
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    [webView setUIDelegate:uiDelegate.get()];
+
+    [webView synchronouslyLoadTestPageNamed:@"beforeunload"];
+
+    TestWebKitAPI::Util::spinRunLoop(10);
+
+    [webView _tryClose];
+
+    // The beforeunload event handler should get called even if the client application does not
+    // have a UIDelegate that can show the before unload prompt.
+    TestWebKitAPI::Util::run(&wasBeforeUnloadEventHandlerCalled);
+
+    // The view should get closed.
+    while (![webView _isClosed])
+        TestWebKitAPI::Util::sleep(0.1);
+}