Investigate improvement to JavaScript alert dequeueing

[data-sync-user]

As discussed in https://github.com/mozilla-mobile/firefox-ios/pull/23757#discussion_r1904514229 and https://docs.google.com/document/d/1g4xn4CSDBqS5zfb7PdDIJES6iO8pQ_uKscTCxx_tvDE/edit?tab=t.0

✅ Ticket Action Items

Remove JS alert dequeue called from BVC viewDidLayoutSubviews and move it elsewhere

1. This UI func is not a good place to be performing an alert dequeue
2. It was added there as a way to dequeue the alert. It’s a way to smash that dequeue alert function until one of them is called at the right time and place, since we don’t have a reliable way to detect when BVC is shown if we, for example, select a tab from the tab tray and an alert should be shown.
3. We cannot rely at the moment on viewDidAppear and viewWillAppear methods in BVC to know when the BVC is shown to the user. Example is when the tab tray is shown on iPhone, those lifecycle methods are not called since the tab tray is not shown full screen.
4. tabManager(:didSelectedTabChange) is too early and we cannot show the alert since the tab tray is still on screen when the alert is attempted to be shown, which result in this error: Attempt to present <Client.NewJSPromptAlertController: 0x12c808e00> on <Client.RootNavigationController: 0x10e809c00> (from <Client.BrowserViewController: 0x10d82ce00>) which is already presenting <Client.DismissableNavigationViewController: 0x10d940800>

┆Issue is synchronized with this Jira Task

[cavaevinci]

@FilippoZazzeroni Hi, I’ve been looking into removing the JS alert dequeue from viewDidLayoutSubviews in BrowserViewController (BVC) as requested.What do we think about to use a notification posted when the tab tray (or any modal) dismisses, combined with a check to ensure BVC is the topmost, unobstructed view.So like :

• Remove dequeue from viewDidLayoutSubviews.
• Post a notification when modals (like TabTrayViewController) dismiss.
• Listen for this in BVC and dequeue alerts only when it’s safe.

[FilippoZazzeroni]

Hi @cavaevinci this task is quite tricky, i'd have to see the code. The JS alert handling was a top crash for us and thanks to @lmarceau and @mattreaganmozilla we managed to solve it. Anyway i suggest opening a PR for it so we can go over the code together.

[data-sync-user]

➤ Laurie Marceau commented:

Latest update to this ticket:

We needed to migrate the JavaScript alerts, details in +ticket+ ( https://mozilla-hub.atlassian.net/browse/FXIOS-14739 ). While working on this issue, it was noticed that when we swipe tabs +doesn’t dequeue the alerts+ ( #31827 (review) ). We had that problem beforehand, since the viewDidLayoutSubviews isn’t called whenever we swipe tabs but it became apparent we needed to make a fix as part of that work.

The dequeuing got moved temporarily to frontEmbeddedContent. It appears will 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.+ (

firefox-ios/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift

Line 4734 in 1277730

// TODO: FXIOS-14347 This function will be removed when toolbarTranslucencyRefactor is on for everyone

) 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+ ( https://mozilla-hub.atlassian.net/browse/FXIOS-11036 ).