runJavaScriptAlertPanelWithMessage completion handler not called

[data-sync-user]

Sentry report

Completion handler passed to -[Client.BrowserViewController webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:] was not called

Probably relates to this Xcode trace

!Screen Shot 2022-12-15 at 2.09.03 PM.png|width=1171,height=580!

┆Issue is synchronized with this Jira Task

[data-sync-user]

➤ Laurie Marceau commented:

It seems we need to call the completion handler in the two following codepath on the delegate call runJavaScriptAlertPanelWithMessage:

• firefox-ios/Client/Frontend/Browser/BrowserViewController/BrowserViewController+WebViewDelegates.swift

  Lines 64 to 69 in d0e758b

  	func webView(_ webView: WKWebView, runJavaScriptAlertPanelWithMessage message: String, initiatedByFrame frame: WKFrameInfo, completionHandler: @escaping () -> Void) {
  	let messageAlert = MessageAlert(message: message, frame: frame, completionHandler: completionHandler)
  	if shouldDisplayJSAlertForWebView(webView) {
  	present(messageAlert.alertController(), animated: true, completion: nil)
  	} else if let promptingTab = tabManager[webView] {
  	promptingTab.queueJavascriptAlertPrompt(messageAlert)

  ( https://github.com/mozilla-mobile/firefox-ios/blob/d0e758b65500cba64d1a0438834d1d82813dddd8/Client/Frontend/Browser/BrowserViewController/BrowserViewController%2BWebViewDelegates.swift#L64-L69|smart-link )

[data-sync-user]

➤ Laurie Marceau commented:

Has a high occurrence in 108.1, should be fixed as soon as possible.

[data-sync-user]

➤ Laurie Marceau commented:

Had another look, it seems the problem relates to the queueing of JS alerts. When the webview isn’t shown (or there’s something else showing on top) we queue the alert which means the webview completion handler isn’t called right away. I tested in Safari if they do some equivalent of this, and it seems they indeed do some queuing (when you come back to the tab, the JS alert is there).

What I used for test:

JavaScript Alert

Try it

<script> setTimeout(function() { alert("I am an alert box!"); }, 5000); // Delay 5 seconds </script>

[data-sync-user]

➤ Yoana Rios Diaz commented:

Note to QA: The crash was related to Javascript alert being queued on a Tab and not showing once the tab became active again.
STR:

• Open a website that shows an alert with some delay
• Switch tab and wait for the delay time (to queue JS Alert)
• Return to tab with alert

Expected:

• Queue alerts should be shown when Tab becomes active

Actual:

• Queue alerts were not showed causing the crash because completionHandler wasn’t called

[data-sync-user]

➤ Simion Basca commented:

Verifying as fix on v110 (27314)

[data-sync-user]

➤ Simion Basca commented:

Re-opening since the fix for this issue has been reverted.

[data-sync-user]

➤ Yoana Rios Diaz commented:

The fix for this ticket https://mozilla-hub.atlassian.net/browse/FXIOS-5787 ( https://mozilla-hub.atlassian.net/browse/FXIOS-5787|smart-link ) should fix the root cause of this crash too

[data-sync-user]

➤ Andrei Bodea commented:

Verified as fixed on v9000 (29995) with iPhone 13 Pro (15.7.1).

As the https://mozilla-hub.atlassian.net/browse/FXIOS-5787 ( https://mozilla-hub.atlassian.net/browse/FXIOS-5787|smart-link ) was verified.

Yoana Rios Diaz I think we can re-open this in case we see again the crash, is this fine by closing it?

[data-sync-user]

➤ Yoana Rios Diaz commented:

Andrei Bodea yes I think we can close it 🤞 and reopen again if the crash returns