Refactor FXIOS-14739 [Webview delegates] Javascript alerts with async variant

[lmarceau]

📜 Tickets

Jira ticket
Github issue

💡 Description

Trying to make minimal changes only in that area, using continuations instead of completion handlers. This work is needed due to the same reasons explained in #31792.

Testing Instructions

Tested with use cases under this JIRA ticket. There are 6 use cases in total.

📝 Checklist

• I filled in the ticket numbers and a description of my work
• I updated the PR name to follow our PR naming guidelines
• I ensured unit tests pass and wrote tests for new code
• If working on UI, I checked and implemented accessibility (Dynamic Text and VoiceOver)
• If adding telemetry, I read the data stewardship requirements and will request a data review
• If adding or modifying strings, I read the guidelines and will request a string review from l10n
• If needed, I updated documentation and added comments to complex code

[lmarceau]

Build failed due to increased warnings count, but I see duplicates in there. Not sure what's up

/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1077:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure; this is an error in the Swift 6 language mode
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1326:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1077:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure; this is an error in the Swift 6 language mode
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1326:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1077:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure; this is an error in the Swift 6 language mode
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1326:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1077:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure; this is an error in the Swift 6 language mode
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1326:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1077:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure; this is an error in the Swift 6 language mode
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1326:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1077:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure; this is an error in the Swift 6 language mode
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1326:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure
/Users/vagrant/git/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift:1077:21: warning: capture of '\''completionHandler'\'' with non-Sendable type '\''@MainActor (URLSession.AuthChallengeDisposition, URLCredential?) -> Void'\'' in a '\''@Sendable'\'' closure; this is an error in the Swift 6 language mode
[...]

[lmarceau]

FYI It's late here so I'll test the use cases again tomorrow morning! Requesting review now just to kick-start the process forward in the meantime.

[mobiletest-ci-bot]

	Warnings
⚠️	Changes detected in files: firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift, firefox-ios/Client/TabManagement/Tab.swift. Ensure that necessary updates are also ported to the WebEngine project if required (cc @lmarceau).

	Messages
📖	Project coverage: 38.03%

🥇 Perfect PR size

Smaller PRs are easier to review. Thanks for making life easy for reviewers! ✨

💬 Description craftsman

Great PR description! Reviewers salute you 🫡

🦊 BrowserViewController Check

We’re tracking the size of BrowserViewController.swift to keep it healthy.

• ✨ Change in file size: +4 lines

❌ Per-file test coverage gate

The following changed file(s) are below 35.0% coverage:

File	Coverage	Required
BrowserKit/Sources/WebEngine/WKWebview/Alerts/JavaScriptAlerts.swift	0.0%	35.0%
firefox-ios/Client/Frontend/Browser/BrowserPrompts.swift	0.6%	35.0%
firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift	17.9%	35.0%
firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift	28.9%	35.0%

Client.app: Coverage: 37.23

File	Coverage
BrowserViewController.swift	28.88%	⚠️
ShareSheetCoordinator.swift	27.44%	⚠️
BrowserViewController+WebViewDelegates.swift	17.92%	⚠️
Tab.swift	57.64%	✅
BrowserPrompts.swift	0.63%	⚠️

Generated by 🚫 Danger Swift against 1de00d0

[lmarceau]

Went through the tests cases again, all LGTM this is ready for review

[mattreaganmozilla]

This seems like a potentially significant change in how these are being executed, so hopefully @ih-codes can also take a look. Approving, but I haven't had a chance to test the branch yet, I can do so later today. Can you also LMK if we've tested the JS alert spam bug? If not I can do that before we merge.

[ih-codes]

A few thoughts... I'm going to tinker a bit more to see if I can think of a way to rewrite the handler method instead of duplicating the logic. 🤔

Still have to test as well before I approve, but wanted to give this a first pass!

[lmarceau]

@mattreaganmozilla

are we unable to DRY this out into a single function due to the mismatched completion signatures? Or is it because the function itself wasn't efficient?

You can look at the handleJavaScriptAlert I had before removing the DRYing here 🤔 . I had two TODO in there that I didn't like and wanted to get rid of for clarity. If you think this is better I can go back and try it again!

[lmarceau]

I'll get back to this soon, finishing up on the incident. Probably by tomorrow I should be able to give this another go

[ih-codes]

During testing, I tried Test Case 3 from the ticket. Seems after I tab switch via swipe to add a new homepage tab, and then swipe to go back the tab after 5 seconds, it is in a glitched state. The screen is blank. But if I open the tab manager to re-visit the same tab, everything starts working again.

BUT... on main I just don't see the alerts at all? So... I'm sure something was glitchy with the swipe switing tabs to begin with, but it looks slightly worse here...?

Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-01-29.at.13.20.59.mov

All the other test cases checked out as far as I could tell. We should update the main ticket with all the test cases + @mattreaganmozilla's prompt spam case as well just so QA knows where to look.

[mattreaganmozilla]

We should update the main ticket with all the test cases + @mattreaganmozilla's prompt spam case as well just so QA knows where to look.

FYI I think we're ok with the prompt spam, I tested the branch and looked good to me.

[lmarceau]

As Isabella commented, indeed when we swipe tabs we're not dequeueing the alerts.

We had that problem before this PR, since the viewDidLayoutSubviews isn’t called whenever we swipe tabs. We already knew we were not dequeuing properly, but it became urgent to make an adjustment since with the async change it appears the webview is effectively awaiting the alert to be shown and won't render any content until it's shown to the user.

This is why I am currently moving the JS alert dequeuing temporarily to frontEmbeddedContent. It appears we'll be able to have the dequeuing of the alert done from the redux update (inside updateContentInHomePanel) whenever the updateEmbeddedContent method will go away with toolbar translucency. We need to wait for that change since browserDelegate?.show(webView: webView) is called in two places at the moment. This work is tracked in this ticket.

[lmarceau]

Ready for another review! Cleaned up with the comments + made a fix as explained above. @mattreaganmozilla I didn't DRY it out, as per Isabella's investigation. Let me know if this isn't satisfactory!

[ih-codes]

Thanks for the detailed explanation and followup ticket @lmarceau!

I pulled down and tested all the cases in the ticket once more after your change. I'm not seeing that issue with the swiping tabs anymore. 😃

I will approve since we made a ticket to revisit that later. By the way, CI failed with testPopUpBlocker. I'll retrigger it for you just in case it was a one-time thing. I noticed that it passed locally for me. 🤔

/Users/vagrant/git/firefox-ios/firefox-ios-tests/Tests/XCUITests/BaseTestCase.swift:224 - failed - Timed out waiting for element "AddressToolbar.address" TextField to contain value [example.com](https://example.com/)

[ih-codes]

We've replaced them

I must have given you documentation with a typo, I'm sorry 🥹 (Don't waste your time fixing unless you commit for something else lol)

[lmarceau]

I'll make this change in another PR!

[github-actions]

🚀 PR merged to main, targeting version: 147.5