-
-
Notifications
You must be signed in to change notification settings - Fork 86
feat: ability to open multiple webviews and communicate with them #418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis PR introduces multi-instance webview support to the InAppBrowser plugin, enabling opening multiple webviews simultaneously and targeting commands or messages to specific webviews via unique IDs or broadcasting to all. The openWebView method now returns a Promise resolving to an object containing an id, and core API methods (executeScript, postMessage, setUrl, reload, updateDimensions, etc.) accept an optional id parameter for per-webview scoping. Event listeners are updated to include optional id fields identifying the source webview. Changes
Sequence DiagramsequenceDiagram
actor User
participant App
participant Plugin
participant WV1 as WebView 1<br/>(id: abc123)
participant WV2 as WebView 2<br/>(id: xyz789)
participant Events
User->>App: Call openWebView()
App->>Plugin: openWebView(options)
Plugin->>WV1: Create & register
Plugin->>Events: Register new instance
Plugin-->>App: Promise<{ id: 'abc123' }>
User->>App: Call openWebView() again
App->>Plugin: openWebView(options)
Plugin->>WV2: Create & register
Plugin->>Events: Register new instance
Plugin-->>App: Promise<{ id: 'xyz789' }>
User->>App: postMessage(detail, id:'abc123')
App->>Plugin: postMessage { detail, id }
Plugin->>WV1: Send message
WV1->>Events: emit messageFromWebview
Events-->>App: { id: 'abc123', detail, ... }
User->>App: postMessage(detail) - no id
App->>Plugin: postMessage { detail }
Plugin->>WV1: Broadcast message
Plugin->>WV2: Broadcast message
WV1->>Events: emit messageFromWebview
WV2->>Events: emit messageFromWebview
Events-->>App: { id: 'abc123', ... }
Events-->>App: { id: 'xyz789', ... }
User->>App: close(id: 'abc123')
App->>Plugin: close { id: 'abc123' }
Plugin->>WV1: Dismiss
Plugin->>Events: Unregister instance
Plugin-->>App: Promise<void>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… one or broadcast to all
3aaa332 to
23934ff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
android/src/main/java/ee/forgr/capacitor_inappbrowser/WebViewDialog.java (2)
116-156: AddvolatiletoinstanceIdfor thread-safe visibility across UI and networking threads.The field is written on the UI thread (via
setInstanceId()at InAppBrowserPlugin:776) and read on the WebView's networking thread (at WebViewDialog:2407 inshouldInterceptRequest). WhilesetInstanceId()is always called before the dialog is presented, the lack ofvolatileprovides no memory visibility guarantee for cross-thread reads. ThegetInstanceId()getter may also be called from arbitrary threads, makingvolatilenecessary.Proposed fix
- private String instanceId = ""; + private volatile String instanceId = "";
2369-2423: Consider escapinginstanceIdfor defensive JS injection protection.While
instanceIdis currently always a UUID (which cannot contain single quotes or backslashes), escaping it before embedding into single-quoted JS strings would be a defensive practice if the source ever changes. The proposed escaping is reasonable for future-proofing:Proposed fix
- String dialogId = instanceId != null ? instanceId : ""; + String dialogId = instanceId != null ? instanceId : ""; + // Defensive escaping for JS single-quoted literal context + dialogId = dialogId.replace("\\", "\\\\").replace("'", "\\'");ios/Sources/InAppBrowserPlugin/WKWebViewController.swift (1)
126-533: Fix unconditional overwriting of"id"in event payloads.
Thepayload(with:)function at lines 521–527 always setspayload["id"] = instanceId, which overwrites any"id"field from the webview message. This causes silent data loss when JavaScript sendswindow.mobileApp.postMessage({ id: "custom-id", ... })— the user-provided id is replaced with the instance id.The TypeScript API expects
idto identify the webview instance anddetailto hold the message payload. Apply one of the suggested fixes:
- Preserve existing user-provided id (safer): Only inject instance id if not already present, otherwise use a separate key.
- Use dedicated key (clearer API alignment): Move the instance identifier to
"webViewId"to match the expected structure.This affects
messageFromWebview(lines 539, 542) and potentially other events where user data is emitted.Safer option (preserve user "id" if present)
private func payload(with data: [String: Any] = [:]) -> [String: Any] { guard !instanceId.isEmpty else { return data } var payload = data - payload["id"] = instanceId + if payload["id"] == nil { + payload["id"] = instanceId + } else { + payload["webViewId"] = instanceId + } return payload }android/src/main/java/ee/forgr/capacitor_inappbrowser/InAppBrowserPlugin.java (1)
296-303:currentUrlupdated unconditionally regardless of target.
currentUrlshould only be updated when the target dialog is the active webview, maintaining consistency with howurlChangeEventupdates it (line 633-635).🐛 Proposed fix
String targetId = call.getString("id"); WebViewDialog webViewDialog = resolveDialog(targetId); if (webViewDialog == null) { call.reject("WebView is not initialized"); return; } - currentUrl = url; + if (targetId == null || targetId.equals(activeWebViewId)) { + currentUrl = url; + }
🤖 Fix all issues with AI agents
In
`@android/src/main/java/ee/forgr/capacitor_inappbrowser/InAppBrowserPlugin.java`:
- Around line 332-335: The code calls call.reject("Invalid URL") but then
continues execution and sets currentUrl = url; to fix, stop execution after
rejecting the call by returning immediately (or otherwise short-circuit) so
currentUrl is not set and no custom tab is launched when url is empty; update
the block around call.reject in InAppBrowserPlugin (the method that handles
opening the URL, referencing call.reject and currentUrl) to return right after
calling call.reject (or use an else branch) to prevent further processing.
- Around line 64-66: The fields webViewDialogs and webViewStack are using
non-thread-safe HashMap and ArrayDeque but are accessed from multiple threads
(e.g., runOnUiThread and plugin threads) which can cause race conditions and
ConcurrentModificationException (notably in postMessage broadcast). Replace the
Map and Deque implementations by using ConcurrentHashMap for webViewDialogs and
ConcurrentLinkedDeque for webViewStack, add the corresponding imports
(java.util.concurrent.ConcurrentHashMap and
java.util.concurrent.ConcurrentLinkedDeque), and review any iterations (such as
in postMessage and any methods touching activeWebViewId) to rely on the
thread-safe semantics or use atomic operations/synchronization if needed when
reading/updating activeWebViewId.
In `@ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swift`:
- Around line 132-144: presentView(webViewId:isAnimated:) is resolving
currentPluginCall causing double-resolve with openWebView/open; remove the
resolve from presentView and instead make it either accept a completion callback
or return when presentation finishes so callers (openWebView and open) perform
the single resolve once; update openWebView/open to resolve exactly once with
the web view id (include resolvedId) and to defer that resolve until
presentation completes when isPresentAfterPageLoad is true, and ensure
currentPluginCall?.reject remains only where errors occur.
- Around line 74-131: When any operation method receives a specific webViewId
(e.g., presentView(webViewId:), reload(webViewId:), and other methods that
accept a webViewId), update the active webview references so subsequent calls
without an id target that webview: set activeWebViewId = id, then set
self.webViewController = webViewControllers[id] and
self.navigationWebViewController = navigationControllers[id]; only perform these
assignments if the id is non-nil and the corresponding controller(s) exist
(otherwise leave existing active references unchanged or handle gracefully).
Apply this same pattern inside presentView, reload, and any other methods that
accept a webViewId parameter.
In `@README.md`:
- Around line 640-646: The Type Aliases section incorrectly defines
ClearCookieOptions as Omit<HttpCookie, 'key' | 'value'>; update that alias to
match the real TypeScript interface (see src/definitions.ts): ClearCookieOptions
should include optional id: string and required url: string (or remove the
erroneous alias entirely and reference the documented interface), and ensure any
mention of HttpCookie or omitted fields is removed so the README's Type Aliases
and Interfaces sections are consistent.
In `@src/definitions.ts`:
- Around line 662-665: The declared type for openWebView in the definitions
(openWebView(options: OpenWebViewOptions): Promise<{ id: string }>) doesn't
match the implementation in web.ts which returns Promise<any>; update the
implementation of openWebView in src/web.ts to explicitly return Promise<{ id:
string }> (adjust the function signature and returned value shape to { id:
string }), update any intermediate return points to construct/return an object
with an id string, and fix any local variables or helper functions used by
openWebView so their outputs conform to this shape; alternatively, if the wider
API requires a different shape, revert the interface change so both declaration
and implementation match.
🧹 Nitpick comments (4)
ios/Sources/InAppBrowserPlugin/WKWebViewController.swift (1)
962-964:websiteDataStore()exposure is useful; confirm it matches your cookie/cache semantics.
This enables per-instance cookie/cache ops in the plugin; just ensure the chosenWKWebsiteDataStore(default vs nonPersistent) aligns with how each webview is configured.ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swift (1)
182-225: Cookie clearing per-id looks reasonable; ensure “no id” behavior matches your contract.
dataStores(for: nil)currently aggregates all instance data stores (or falls back toWKWebsiteDataStore.default()when no controllers exist). That’s fine if “no id” means “all/ambient”, but it’s worth confirming this is what you want forclearCookies.android/src/main/java/ee/forgr/capacitor_inappbrowser/InAppBrowserPlugin.java (2)
976-1005: Redundant null check insiderunOnUiThread.
dialogis verified non-null at line 962 before reachingrunOnUiThread. The check at line 981 is always true. The else branch (lines 1007-1017) is unreachable dead code.♻️ Simplify by removing unreachable code
try { - if (dialog != null) { String currentUrl = ""; try { currentUrl = dialog.getUrl(); if (currentUrl == null) { currentUrl = ""; } } catch (Exception e) { Log.e("InAppBrowser", "Error getting URL before close: " + e.getMessage()); currentUrl = ""; } // Notify listeners about the close event JSObject eventData = new JSObject().put("url", currentUrl); if (targetId != null) { eventData.put("id", targetId); } notifyListeners("closeEvent", eventData); dialog.dismiss(); if (targetId != null) { unregisterWebView(targetId); } else { webViewDialog = null; } call.resolve(); - } else { - // Secondary fallback inside UI thread - try { - Intent intent = new Intent(getContext(), getBridge().getActivity().getClass()); - intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP); - getContext().startActivity(intent); - call.resolve(); - } catch (Exception e) { - Log.e("InAppBrowser", "Error in secondary fallback: " + e.getMessage()); - call.reject("WebView is not initialized"); - } - } } catch (Exception e) {
833-839: Address the TODO: Legacy fallback code.The
webViewDialogfallback at lines 833-839 (and similarly at 878-884 inexecuteScript) appears redundant given thatregisterWebViewalready updateswebViewDialogto match the active dialog. Consider removing this dead code path or clarifying when it would be reached.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.mdandroid/src/main/java/ee/forgr/capacitor_inappbrowser/InAppBrowserPlugin.javaandroid/src/main/java/ee/forgr/capacitor_inappbrowser/WebViewDialog.javaios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftios/Sources/InAppBrowserPlugin/WKWebViewController.swiftsrc/definitions.ts
🧰 Additional context used
🧬 Code graph analysis (1)
ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swift (1)
ios/Sources/InAppBrowserPlugin/WKWebViewController.swift (18)
webView(1586-1620)webView(1622-1643)webView(1783-1790)webView(1791-1832)webView(1834-1842)webView(1844-1852)webView(1854-1879)webView(1881-1939)websiteDataStore(962-964)goBack(1412-1418)reload(958-960)load(928-937)load(939-943)load(945-947)load(949-951)executeScript(966-970)postMessageToJS(502-519)cleanupWebView(1009-1035)
🔇 Additional comments (9)
ios/Sources/InAppBrowserPlugin/WKWebViewController.swift (1)
917-918: Centralizedemit(...)routing looks good for multi-instance event attribution.
The move toemit(...)+instanceIdpropagation is a clean way to keep events instance-scoped (URL change, errors, button actions, close flows).Also applies to: 1294-1295, 1424-1426, 1476-1480, 1515-1519, 1547-1550, 1802-1807, 1831-1852, 1926-1927
ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swift (1)
98-131: Targeted vs broadcast behavior is implemented consistently forexecuteScript/postMessage.
The “id => targeted, no id => broadcast (when multiple exist)” behavior matches the issue’s goal for inter-webview messaging.Also applies to: 860-883, 895-919
android/src/main/java/ee/forgr/capacitor_inappbrowser/InAppBrowserPlugin.java (3)
487-488: LGTM: UUID-based webview ID generation and registration.The unique ID generation and registration flow is well-structured. Each webview gets a UUID, the dialog is registered with proper stack management, and the ID is returned to the caller.
Also applies to: 776-780
82-101: LGTM: Webview registration and unregistration logic.The stack-based management with
webViewStackand theactiveWebViewIdtracking is well thought out. Unregistration properly updates the active ID to the previous webview in the stack.
1074-1090: LGTM: Comprehensive cleanup inhandleOnDestroy.All dialogs are properly dismissed with exception handling, and all multi-webview state is cleared. This ensures no resource leaks on plugin destruction.
src/definitions.ts (2)
611-682: LGTM: Consistent API surface for multi-webview support.The TypeScript definitions properly expose the
idparameter as optional across all relevant methods (goBack,clearCookies,clearAllCookies,clearCache,close,executeScript,postMessage,setUrl,reload,updateDimensions). The documentation clearly explains the behavior whenidis omitted (broadcast or target active).
712-731: LGTM: Event payloads include webview ID.The
messageFromWebview,browserPageLoaded, andpageLoadErrorlisteners now receive anidfield identifying the source webview. This enables proper event handling in multi-webview scenarios.README.md (2)
51-67: LGTM: Clear documentation for multi-webview usage.The example demonstrates the proper pattern: destructuring the
idfromopenWebView, then passing it to subsequent calls likeupdateDimensions. The comment on line 61 clarifies the optional nature and fallback behavior.
167-171: LGTM: PostMessage examples show both targeted and broadcast patterns.The documentation clearly demonstrates sending a message to a specific webview vs. broadcasting to all open webviews.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| private final Map<String, WebViewDialog> webViewDialogs = new HashMap<>(); | ||
| private final Deque<String> webViewStack = new ArrayDeque<>(); | ||
| private String activeWebViewId = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thread safety concern with shared mutable state.
HashMap and ArrayDeque are not thread-safe. These collections are accessed from both the main thread (via runOnUiThread) and plugin method threads. This can cause race conditions or ConcurrentModificationException when iterating (e.g., in postMessage broadcast).
🔒 Recommended fix: Use thread-safe collections
- private final Map<String, WebViewDialog> webViewDialogs = new HashMap<>();
- private final Deque<String> webViewStack = new ArrayDeque<>();
- private String activeWebViewId = null;
+ private final Map<String, WebViewDialog> webViewDialogs = new ConcurrentHashMap<>();
+ private final Deque<String> webViewStack = new ConcurrentLinkedDeque<>();
+ private volatile String activeWebViewId = null;Add the imports:
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedDeque;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private final Map<String, WebViewDialog> webViewDialogs = new HashMap<>(); | |
| private final Deque<String> webViewStack = new ArrayDeque<>(); | |
| private String activeWebViewId = null; | |
| private final Map<String, WebViewDialog> webViewDialogs = new ConcurrentHashMap<>(); | |
| private final Deque<String> webViewStack = new ConcurrentLinkedDeque<>(); | |
| private volatile String activeWebViewId = null; |
🤖 Prompt for AI Agents
In
`@android/src/main/java/ee/forgr/capacitor_inappbrowser/InAppBrowserPlugin.java`
around lines 64 - 66, The fields webViewDialogs and webViewStack are using
non-thread-safe HashMap and ArrayDeque but are accessed from multiple threads
(e.g., runOnUiThread and plugin threads) which can cause race conditions and
ConcurrentModificationException (notably in postMessage broadcast). Replace the
Map and Deque implementations by using ConcurrentHashMap for webViewDialogs and
ConcurrentLinkedDeque for webViewStack, add the corresponding imports
(java.util.concurrent.ConcurrentHashMap and
java.util.concurrent.ConcurrentLinkedDeque), and review any iterations (such as
in postMessage and any methods touching activeWebViewId) to rely on the
thread-safe semantics or use atomic operations/synchronization if needed when
reading/updating activeWebViewId.
| if (TextUtils.isEmpty(url)) { | ||
| call.reject("Invalid URL"); | ||
| } | ||
| currentUrl = url; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing return after call.reject.
When the URL is empty, call.reject is called but execution continues, which will use the empty URL for currentUrl and proceed to launch the custom tab.
🐛 Proposed fix
if (TextUtils.isEmpty(url)) {
call.reject("Invalid URL");
+ return;
}
currentUrl = url;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (TextUtils.isEmpty(url)) { | |
| call.reject("Invalid URL"); | |
| } | |
| currentUrl = url; | |
| if (TextUtils.isEmpty(url)) { | |
| call.reject("Invalid URL"); | |
| return; | |
| } | |
| currentUrl = url; |
🤖 Prompt for AI Agents
In
`@android/src/main/java/ee/forgr/capacitor_inappbrowser/InAppBrowserPlugin.java`
around lines 332 - 335, The code calls call.reject("Invalid URL") but then
continues execution and sets currentUrl = url; to fix, stop execution after
rejecting the call by returning immediately (or otherwise short-circuit) so
currentUrl is not set and no custom tab is launched when url is empty; update
the block around call.reject in InAppBrowserPlugin (the method that handles
opening the URL, referencing call.reject and currentUrl) to return right after
calling call.reject (or use an else branch) to prevent further processing.
| private func registerWebView(id: String, webView: WKWebViewController, navigationController: UINavigationController) { | ||
| webViewControllers[id] = webView | ||
| navigationControllers[id] = navigationController | ||
| webViewStack.removeAll { $0 == id } | ||
| webViewStack.append(id) | ||
| activeWebViewId = id | ||
| self.webViewController = webView | ||
| self.navigationWebViewController = navigationController | ||
| } | ||
|
|
||
| private func unregisterWebView(id: String) { | ||
| webViewControllers[id] = nil | ||
| navigationControllers[id] = nil | ||
| webViewStack.removeAll { $0 == id } | ||
| activeWebViewId = webViewStack.last | ||
| if let activeId = activeWebViewId { | ||
| self.webViewController = webViewControllers[activeId] | ||
| self.navigationWebViewController = navigationControllers[activeId] | ||
| } else { | ||
| self.webViewController = nil | ||
| self.navigationWebViewController = nil | ||
| } | ||
| } | ||
|
|
||
| private func resolveWebViewController(for id: String?) -> WKWebViewController? { | ||
| if let id { | ||
| return webViewControllers[id] | ||
| } | ||
| return webViewController | ||
| } | ||
|
|
||
| private func dataStores(for targetId: String?) -> [WKWebsiteDataStore] { | ||
| if let targetId { | ||
| guard let controller = webViewControllers[targetId], | ||
| let store = controller.websiteDataStore() else { | ||
| return [] | ||
| } | ||
| return [store] | ||
| } | ||
|
|
||
| let controllers = Array(webViewControllers.values) | ||
| if controllers.isEmpty { | ||
| return [WKWebsiteDataStore.default()] | ||
| } | ||
|
|
||
| var seen = Set<ObjectIdentifier>() | ||
| var stores: [WKWebsiteDataStore] = [] | ||
| for controller in controllers { | ||
| if let store = controller.websiteDataStore() { | ||
| let identifier = ObjectIdentifier(store) | ||
| if seen.insert(identifier).inserted { | ||
| stores.append(store) | ||
| } | ||
| } | ||
| } | ||
| return stores | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's get the file size and line count
wc -l ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftRepository: Cap-go/capacitor-inappbrowser
Length of output: 133
🏁 Script executed:
# Now let's look at the presentView method (around lines 132-141)
sed -n '120,155p' ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftRepository: Cap-go/capacitor-inappbrowser
Length of output: 1514
🏁 Script executed:
# Look at the close methods (around lines 807-831)
sed -n '800,840p' ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftRepository: Cap-go/capacitor-inappbrowser
Length of output: 1441
🏁 Script executed:
# Look at the reload method (around lines 1024-1051)
sed -n '1015,1060p' ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftRepository: Cap-go/capacitor-inappbrowser
Length of output: 1954
🏁 Script executed:
# Search for methods that take an "id" parameter
grep -n "getString.*id" ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swift | head -20Repository: Cap-go/capacitor-inappbrowser
Length of output: 592
🏁 Script executed:
# Look at the full close method to understand its structure better
sed -n '1024,1060p' ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftRepository: Cap-go/capacitor-inappbrowser
Length of output: 1730
🏁 Script executed:
# Check methods around lines 809, 822 to understand what they are
sed -n '805,835p' ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftRepository: Cap-go/capacitor-inappbrowser
Length of output: 1125
🏁 Script executed:
# Let me also check the other methods that use getString("id")
sed -n '1110,1125p' ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftRepository: Cap-go/capacitor-inappbrowser
Length of output: 732
🏁 Script executed:
# Check what the other methods at the "Also applies to" lines are
sed -n '188,195p' ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftRepository: Cap-go/capacitor-inappbrowser
Length of output: 324
🏁 Script executed:
# Check the context around line 860 and 896 to understand those methods
sed -n '858,870p' ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftRepository: Cap-go/capacitor-inappbrowser
Length of output: 547
🏁 Script executed:
sed -n '894,906p' ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftRepository: Cap-go/capacitor-inappbrowser
Length of output: 544
Update activeWebViewId when presenting or operating on a specific webview id, to ensure subsequent operations target the correct webview.
Currently, activeWebViewId only changes when webviews are registered/unregistered. Methods like presentView() and reload() accept a specific webViewId parameter but don't update activeWebViewId, causing subsequent calls without an id to target the last-registered webview instead of the currently visible/operated-on one.
For example: if you call presentView(webViewId: "A"), then later call reload() without an id, the reload targets the previously active webview rather than webview "A".
The close() method correctly handles this via unregisterWebView(), but presentView(), reload(), and other operation methods need similar treatment. When any of these methods receive a specific webViewId, they should update activeWebViewId and the active controller references accordingly.
Proposed fix for presentView
func presentView(webViewId: String? = nil, isAnimated: Bool = true) {
let resolvedId = webViewId ?? activeWebViewId
+ if let resolvedId {
+ webViewStack.removeAll { $0 == resolvedId }
+ webViewStack.append(resolvedId)
+ activeWebViewId = resolvedId
+ self.webViewController = webViewControllers[resolvedId]
+ self.navigationWebViewController = navigationControllers[resolvedId]
+ }
let navigationController = resolvedId.flatMap { navigationControllers[$0] } ?? self.navigationWebViewController
...
}Apply similar updates to reload() (line 822) and other methods that accept a specific webViewId.
🤖 Prompt for AI Agents
In `@ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swift` around lines 74 -
131, When any operation method receives a specific webViewId (e.g.,
presentView(webViewId:), reload(webViewId:), and other methods that accept a
webViewId), update the active webview references so subsequent calls without an
id target that webview: set activeWebViewId = id, then set
self.webViewController = webViewControllers[id] and
self.navigationWebViewController = navigationControllers[id]; only perform these
assignments if the id is non-nil and the corresponding controller(s) exist
(otherwise leave existing active references unchanged or handle gracefully).
Apply this same pattern inside presentView, reload, and any other methods that
accept a webViewId parameter.
| func presentView(webViewId: String? = nil, isAnimated: Bool = true) { | ||
| let resolvedId = webViewId ?? activeWebViewId | ||
| let navigationController = resolvedId.flatMap { navigationControllers[$0] } ?? self.navigationWebViewController | ||
| guard let navigationController else { | ||
| self.currentPluginCall?.reject("Navigation controller is not initialized") | ||
| return | ||
| } | ||
|
|
||
| self.bridge?.viewController?.present(navigationController, animated: isAnimated, completion: { | ||
| let presenter = self.bridge?.viewController?.presentedViewController ?? self.bridge?.viewController | ||
| presenter?.present(navigationController, animated: isAnimated, completion: { | ||
| self.currentPluginCall?.resolve() | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix double-resolve (and clarify openWebView resolve timing vs isPresentAfterPageLoad).
presentView(...) resolves currentPluginCall, but openWebView (and open) also call call.resolve(...) in their own flow, which can resolve the same Capacitor call twice. It also makes openWebView resolve immediately even when isPresentAfterPageLoad is true.
One workable approach (keep “resolve when presented” semantics, include id)
public class InAppBrowserPlugin: CAPPlugin, CAPBridgedPlugin {
+ private var pendingOpenResult: [String: Any]?
func presentView(webViewId: String? = nil, isAnimated: Bool = true) {
let resolvedId = webViewId ?? activeWebViewId
let navigationController = resolvedId.flatMap { navigationControllers[$0] } ?? self.navigationWebViewController
guard let navigationController else {
self.currentPluginCall?.reject("Navigation controller is not initialized")
return
}
let presenter = self.bridge?.viewController?.presentedViewController ?? self.bridge?.viewController
presenter?.present(navigationController, animated: isAnimated, completion: {
- self.currentPluginCall?.resolve()
+ self.currentPluginCall?.resolve(self.pendingOpenResult ?? [:])
+ self.pendingOpenResult = nil
+ self.currentPluginCall = nil
})
}
`@objc` func openWebView(_ call: CAPPluginCall) {
...
self.currentPluginCall = call
...
let webViewId = UUID().uuidString
+ self.pendingOpenResult = ["id": webViewId]
...
DispatchQueue.main.async {
...
if !self.isPresentAfterPageLoad {
self.presentView(webViewId: webViewId, isAnimated: isAnimated)
}
- call.resolve(["id": webViewId])
+ // resolved by presentView completion (immediate if presented now, delayed if presentAfterPageLoad)
}
}
`@objc` func open(_ call: CAPPluginCall) {
...
self.currentPluginCall = call
+ self.pendingOpenResult = [:]
...
DispatchQueue.main.async {
...
if !self.isPresentAfterPageLoad {
self.presentView()
}
- call.resolve()
+ // resolved by presentView completion
}
}
}Repo verification script
#!/bin/bash
set -euo pipefail
# Look for other multi-resolve patterns introduced by presentView() resolving currentPluginCall.
rg -n --type=swift 'currentPluginCall\?\.(resolve|reject)\b|call\.resolve\b|presentView\(' ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swiftAlso applies to: 251-267, 800-804, 938-1021
🤖 Prompt for AI Agents
In `@ios/Sources/InAppBrowserPlugin/InAppBrowserPlugin.swift` around lines 132 -
144, presentView(webViewId:isAnimated:) is resolving currentPluginCall causing
double-resolve with openWebView/open; remove the resolve from presentView and
instead make it either accept a completion callback or return when presentation
finishes so callers (openWebView and open) perform the single resolve once;
update openWebView/open to resolve exactly once with the web view id (include
resolvedId) and to defer that resolve until presentation completes when
isPresentAfterPageLoad is true, and ensure currentPluginCall?.reject remains
only where errors occur.
| #### ClearCookieOptions | ||
|
|
||
| | Prop | Type | | ||
| | --------- | ------------------- | | ||
| | **`url`** | <code>string</code> | | ||
| | Prop | Type | Description | | ||
| | --------- | ------------------- | -------------------------------------------------------------- | | ||
| | **`id`** | <code>string</code> | Target webview id. When omitted, applies to all open webviews. | | ||
| | **`url`** | <code>string</code> | | | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ClearCookieOptions definitions in TypeScript files
echo "=== Searching for ClearCookieOptions in .ts files ==="
rg -n "ClearCookieOptions" --type ts -B 2 -A 5
echo -e "\n=== Searching for HttpCookie definition ==="
rg -n "HttpCookie" --type ts -B 2 -A 5 | head -60Repository: Cap-go/capacitor-inappbrowser
Length of output: 1502
🏁 Script executed:
#!/bin/bash
# Check README.md at the specified lines
echo "=== README.md lines 640-646 ==="
sed -n '640,646p' README.md
echo -e "\n=== README.md lines 778-781 ==="
sed -n '778,781p' README.md
echo -e "\n=== Broader search for HttpCookie ==="
rg -n "HttpCookie" -B 2 -A 3
echo -e "\n=== Check for docgen files/scripts ==="
find . -type f \( -name "*docgen*" -o -name "*doc*" \) | grep -E "\.(js|ts|json|sh)$" | head -20Repository: Cap-go/capacitor-inappbrowser
Length of output: 1461
🏁 Script executed:
#!/bin/bash
# Check if url is used with ClearCookieOptions anywhere
echo "=== Search for url usage in ClearCookies context ==="
rg -n "url" -B 5 -A 5 | grep -A 10 -B 10 -i "clearcookie"
echo -e "\n=== Full ClearCookieOptions definition ==="
sed -n '68,75p' src/definitions.ts
echo -e "\n=== Check for any .d.ts or generated files ==="
find . -name "*.d.ts" | head -10
</script>Repository: Cap-go/capacitor-inappbrowser
Length of output: 9489
Remove or correct the Type Aliases section definition for ClearCookieOptions.
The Interfaces section (lines 640-646) correctly documents ClearCookieOptions with id and url fields, matching the actual TypeScript definition in src/definitions.ts. However, the Type Aliases section (lines 778-781) incorrectly defines it as Omit<HttpCookie, 'key' | 'value'>, which contradicts both the actual interface and the Interfaces section. The url field is required by both the native implementations (Android and iOS) and the TypeScript interface, so it must be documented in all README sections consistently.
Also applies to: 778-781
🤖 Prompt for AI Agents
In `@README.md` around lines 640 - 646, The Type Aliases section incorrectly
defines ClearCookieOptions as Omit<HttpCookie, 'key' | 'value'>; update that
alias to match the real TypeScript interface (see src/definitions.ts):
ClearCookieOptions should include optional id: string and required url: string
(or remove the erroneous alias entirely and reference the documented interface),
and ensure any mention of HttpCookie or omitted fields is removed so the
README's Type Aliases and Interfaces sections are consistent.
| * @returns Promise that resolves with the created webview id. | ||
| * @since 0.1.0 | ||
| */ | ||
| openWebView(options: OpenWebViewOptions): Promise<any>; | ||
| openWebView(options: OpenWebViewOptions): Promise<{ id: string }>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if src/definitions.ts exists and look at the openWebView definition
if [ -f "src/definitions.ts" ]; then
echo "=== openWebView definition around lines 662-665 ==="
sed -n '660,670p' src/definitions.ts
echo ""
fi
# Check for all openWebView usages in the codebase
echo "=== openWebView usages in TypeScript/JavaScript files ==="
rg -n "openWebView" --type ts --type js -B 1 -A 3 2>/dev/null | head -100Repository: Cap-go/capacitor-inappbrowser
Length of output: 4025
🏁 Script executed:
# Check if there's a previous version of this file in git history to see the old return type
if git rev-parse --git-dir > /dev/null 2>&1; then
echo "=== Git history for openWebView changes ==="
git log --oneline -p -- src/definitions.ts 2>/dev/null | grep -A 5 -B 5 "openWebView" | head -50
fiRepository: Cap-go/capacitor-inappbrowser
Length of output: 1650
Fix: openWebView type definition does not match implementation.
The interface in src/definitions.ts (line 665) declares Promise<{ id: string }>, but the implementation in src/web.ts (line 36) still returns Promise<any>. Update the implementation to match the type definition or revert the interface change.
🤖 Prompt for AI Agents
In `@src/definitions.ts` around lines 662 - 665, The declared type for openWebView
in the definitions (openWebView(options: OpenWebViewOptions): Promise<{ id:
string }>) doesn't match the implementation in web.ts which returns
Promise<any>; update the implementation of openWebView in src/web.ts to
explicitly return Promise<{ id: string }> (adjust the function signature and
returned value shape to { id: string }), update any intermediate return points
to construct/return an object with an id string, and fix any local variables or
helper functions used by openWebView so their outputs conform to this shape;
alternatively, if the wider API requires a different shape, revert the interface
change so both declaration and implementation match.
Multi‑webview support with id‑based targeting and/or broadcast behavior.
Tested with demo-app on iOS and Android to confirm no breaking changes, everything working as expected.
Closes #394
Disclaimer
This PR is mainly AI, reviewed and tested by me
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.