Summary
WKUIController.runJavaScriptConfirmPanelWithMessage (and the alert variant) presents a UIAlertController and calls completionHandler(_:) only from inside the UIAlertAction closures. If the alert is dismissed without any action being triggered — e.g. because the presenting view controller is torn down (deeplink navigation from clicking a push notification, programmatic dismissal, etc.) — the completion handler is never called.
WebKit then crashes the app on the next interaction with the offending web view:
NSInternalInconsistencyException:
Completion handler passed to -[HotwireNative.WKUIController
webView:runJavaScriptConfirmPanelWithMessage:initiatedByFrame:completionHandler:] was not called
In our production app this surfaced as: user has a JS confirm() dialog open, taps an incoming push notification, the deeplink handler switches tabs and pushes a new screen, the alert disappears with no UIAlertAction ever firing, and the next web-view interaction crashes. Reproducing the full push/deeplink flow in the demo would require standing up a notification system we don't need for the root cause, so the repro below uses a 6-second timer in SceneController to perform the same deeplink-style navigation. The crash is identical.
This is a follow-up to #233. Per @joemasilotti's review there, the nil-guard fix for the auth-challenge crash has been kept, but the JS dialog fix was left out. This issue is a clean, scoped repro so we can decide separately whether to fix it upstream.
Reproduction
Patches against hotwire-native-ios (noah44846@cdc2995) and hotwire-native-demo (noah44846/hotwire-native-demo@675986d) below. Set Demo.current = Demo.local and run the Rails app locally so the "Bugs & Fixes" tab is enabled.
Then in the demo iOS app:
- Open the Bugs & Fixes tab → JS confirm dismissed externally.
- Tap Arm deeplink (fires in 6s). The iOS demo schedules a simulated notification deeplink — when the timer fires it switches to the first tab and pushes a new screen onto that navigator.
- Tap Trigger confirm() — the native JS confirm dialog appears.
- Do not tap OK or Cancel. Wait for the timer.
- The simulated deeplink fires, the presenting view controller is dismissed, no
UIAlertAction runs, completion handler is dropped.
- Any subsequent interaction with that web view (or another JS dialog) crashes with the
NSInternalInconsistencyException above.
Patch — hotwire-native-ios (Demo)
diff --git a/Demo/SceneController.swift b/Demo/SceneController.swift
index 36d53d7..52e038b 100644
--- a/Demo/SceneController.swift
+++ b/Demo/SceneController.swift
@@ -15,6 +15,23 @@ final class SceneController: UIResponder {
let authURL = rootURL.appendingPathComponent("/signin")
tabBarController.activeNavigator.route(authURL)
}
+
+ // MARK: - Bug repro: confirm() dismissed externally
+
+ /// Simulates a push-notification deeplink: switches to the first tab and
+ /// pushes a new screen onto its navigator. Used by the "JS confirm
+ /// dismissed externally" bug repro to dismiss the presenting view
+ /// controller (and its alert) while a JS confirm() dialog is on screen.
+ private func simulateNotificationDeeplink() {
+ tabBarController.selectedIndex = 0
+ tabBarController.activeNavigator.route(rootURL.appendingPathComponent("/navigation"))
+ }
+
+ private func scheduleNotificationDeeplink() {
+ DispatchQueue.main.asyncAfter(deadline: .now() + 6) { [weak self] in
+ self?.simulateNotificationDeeplink()
+ }
+ }
}
extension SceneController: UIWindowSceneDelegate {
@@ -31,6 +48,11 @@ extension SceneController: UIWindowSceneDelegate {
extension SceneController: NavigatorDelegate {
func handle(proposal: VisitProposal, from navigator: Navigator) -> ProposalResult {
+ if proposal.url.path == "/bugs/schedule_deeplink" {
+ scheduleNotificationDeeplink()
+ return .reject
+ }
+
switch proposal.viewController {
case NumbersViewController.pathConfigurationIdentifier:
return .acceptCustom(NumbersViewController(
Patch — hotwire-native-demo (Rails)
diff --git a/app/controllers/bugs_controller.rb b/app/controllers/bugs_controller.rb
index 86e049f..7566624 100644
--- a/app/controllers/bugs_controller.rb
+++ b/app/controllers/bugs_controller.rb
@@ -1,2 +1,8 @@
class BugsController < ApplicationController
+ def confirm_dismissal
+ end
+
+ def schedule_deeplink
+ head :no_content
+ end
end
diff --git a/app/views/bugs/index.html.erb b/app/views/bugs/index.html.erb
index 61d5077..8016a8c 100644
--- a/app/views/bugs/index.html.erb
+++ b/app/views/bugs/index.html.erb
@@ -4,13 +4,11 @@
<h1 class="hide@native margin-bs-xl text-large-title">Bugs & Fixes</h1>
<p>An area to reproduce outstanding bugs and ensure no regressions on existing fixes reappear.</p>
- <p><i>No bugs currently being investigated.</i><p>
-
<div class="formatted-list formatted-list--top-level">
- <%# render "navigations/item",
- path: some_bugs_path,
- icon: "custom-bug-icon",
- name: "Bug Title",
- description: "Description of bug." %>
+ <%= render "navigations/item",
+ path: confirm_dismissal_bugs_path,
+ icon: "arrow-right-bold",
+ name: "JS confirm dismissed externally",
+ description: "Reproduce crash when a confirm() dialog is dismissed without any UIAlertAction being triggered." %>
</div>
</div>
diff --git a/config/routes.rb b/config/routes.rb
index b7fcbee..754b2df 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -1,7 +1,8 @@
Rails.application.routes.draw do
resources :bugs, only: :index do
collection do
- # get :some
+ get :confirm_dismissal
+ get :schedule_deeplink
end
end
New view app/views/bugs/confirm_dismissal.html.erb:
<% content_for :title, "JS confirm dismissed externally" %>
<div class="top-level-container">
<h1 class="hide@native margin-bs-xl text-large-title">JS confirm dismissed externally</h1>
<div class="formatted-list formatted-list--top-level">
<%= link_to "Arm deeplink (fires in 6s)",
schedule_deeplink_bugs_path,
class: "formatted-list__item" %>
<%= link_to "Trigger confirm()",
confirm_dismissal_bugs_path,
data: { turbo_method: :delete, turbo_confirm: "Are you sure?" },
class: "formatted-list__item" %>
</div>
<p>... (steps) ...</p>
</div>
Proposed fix (sketch)
Our initial attempt was to subclass UIAlertController and call the completion handler from deinit as a fallback. That works, but Apple's docs explicitly warn against subclassing UIAlertController ("The UIAlertController class is intended to be used as-is and does not support subclassing"), so we don't think it's appropriate for upstream. Two alternatives that don't subclass it:
-
Sentinel via associated object — attach a small object to the alert whose
deinit invokes the completion handler with a safe default if no action ran:
private final class CompletionGuard {
var handler: (() -> Void)?
init(_ handler: @escaping () -> Void) { self.handler = handler }
deinit { handler?() }
}
Attach with objc_setAssociatedObject so the guard's lifetime is tied to the alert; deinit fires whether the alert is dismissed by an action, by external teardown, or by deallocation.
-
Track pending handlers on WKUIController — store them and call any
outstanding ones in deinit / on a tear-down signal. Less defensive (only
covers the WKUIController-teardown path, not arbitrary external dismissal of the alert while the controller is alive).
Happy to put up a PR with whichever direction you prefer — wanted to file the repro and discussion first so the previous PR could merge cleanly.
Related
Summary
WKUIController.runJavaScriptConfirmPanelWithMessage(and the alert variant) presents aUIAlertControllerand callscompletionHandler(_:)only from inside theUIAlertActionclosures. If the alert is dismissed without any action being triggered — e.g. because the presenting view controller is torn down (deeplink navigation from clicking a push notification, programmatic dismissal, etc.) — the completion handler is never called.WebKit then crashes the app on the next interaction with the offending web view:
In our production app this surfaced as: user has a JS
confirm()dialog open, taps an incoming push notification, the deeplink handler switches tabs and pushes a new screen, the alert disappears with noUIAlertActionever firing, and the next web-view interaction crashes. Reproducing the full push/deeplink flow in the demo would require standing up a notification system we don't need for the root cause, so the repro below uses a 6-second timer inSceneControllerto perform the same deeplink-style navigation. The crash is identical.This is a follow-up to #233. Per @joemasilotti's review there, the nil-guard fix for the auth-challenge crash has been kept, but the JS dialog fix was left out. This issue is a clean, scoped repro so we can decide separately whether to fix it upstream.
Reproduction
Patches against
hotwire-native-ios(noah44846@cdc2995) andhotwire-native-demo(noah44846/hotwire-native-demo@675986d) below. SetDemo.current = Demo.localand run the Rails app locally so the "Bugs & Fixes" tab is enabled.Then in the demo iOS app:
UIAlertActionruns, completion handler is dropped.NSInternalInconsistencyExceptionabove.Patch —
hotwire-native-ios(Demo)Patch —
hotwire-native-demo(Rails)New view
app/views/bugs/confirm_dismissal.html.erb:Proposed fix (sketch)
Our initial attempt was to subclass
UIAlertControllerand call the completion handler fromdeinitas a fallback. That works, but Apple's docs explicitly warn against subclassingUIAlertController("TheUIAlertControllerclass is intended to be used as-is and does not support subclassing"), so we don't think it's appropriate for upstream. Two alternatives that don't subclass it:Sentinel via associated object — attach a small object to the alert whose
deinitinvokes the completion handler with a safe default if no action ran:Attach with
objc_setAssociatedObjectso the guard's lifetime is tied to the alert;deinitfires whether the alert is dismissed by an action, by external teardown, or by deallocation.Track pending handlers on
WKUIController— store them and call anyoutstanding ones in
deinit/ on a tear-down signal. Less defensive (onlycovers the
WKUIController-teardown path, not arbitrary external dismissal of the alert while the controller is alive).Happy to put up a PR with whichever direction you prefer — wanted to file the repro and discussion first so the previous PR could merge cleanly.
Related