chore: sync upstream PR #8520 - fix(ios): retain WKHTTPCookieStore observer so cookie sync works#83
chore: sync upstream PR #8520 - fix(ios): retain WKHTTPCookieStore observer so cookie sync works#83riderx wants to merge 23 commits into
Conversation
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
Co-authored-by: Joey Pender <joey.pender@outsystems.com> Co-authored-by: Pedro Bilro <pedro.gustavo.bilro@outsystems.com> Co-authored-by: Mark Anderson <mark.anderson@outsystems.com>
…#8476) Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
…am#8492) Co-authored-by: Mark Anderson <mark.anderson@outsystems.com>
…preferred fallback)
|
Claude Code could not resolve this upstream sync completely. Please review the branch carefully before merging. |
Beta npm buildMaintainers can publish one Capacitor Plus workspace package from this PR to npm for fast testing. Comment Examples: /publish-beta core
/publish-beta cli
/publish-beta @capacitor-plus/coreIf exactly one workspace package changed, Packages:
The workflow will:
Security note: beta publish is only enabled for branches inside this repository. |
📝 WalkthroughWalkthroughThe PR updates the project to version 8.4.1, revises release metadata and CI limits, adds SwiftPM configuration and telemetry behavior to the CLI, changes Android inset handling, and adds Android and iOS configuration and lifecycle updates. ChangesRelease metadata and CI
CLI SwiftPM and telemetry workflows
Platform runtime updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLIConfig
participant generatePackageText
participant ensureSymlink
participant PackageSwift
CLIConfig->>generatePackageText: provide packageOptions
generatePackageText->>ensureSymlink: create configured plugin symlink
generatePackageText->>PackageSwift: emit package path and moduleAliases
sequenceDiagram
participant PluginConfig
participant SystemBars
participant WindowInsetsControllerCompat
PluginConfig->>SystemBars: provide insetsHandling
SystemBars->>SystemBars: validate css or disable
SystemBars->>WindowInsetsControllerCompat: show or hide selected bar types
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: medium. This upstream sync touches iOS, Android, and CLI with merge-conflict resolution and multi-platform behavior changes, exceeding the low-risk approval threshold. Human review is required; reviewers have been assigned.
Sent by Cursor Approval Agent: Pull Request Approver External
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/tasks/run.ts (1)
118-127: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRestore the Android manifest in the live-reload error path.
catchonly revertscapacitor.config.json; for Android it also needs to callwriteCordovaAndroidManifest(..., false)so a failed live-reload run doesn’t leaveAndroidManifest.xmlrewritten.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/tasks/run.ts` around lines 118 - 127, Restore the Android manifest in the live-reload error path within the catch block of the task runner. When options.liveReload is enabled, call writeCordovaAndroidManifest with the appropriate Android project/config arguments and false after reverting the Capacitor config, ensuring failed runs restore AndroidManifest.xml before handling the error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@android/capacitor/src/test/java/com/getcapacitor/plugin/SystemBarsTest.java`:
- Around line 49-69: Update the SystemBarsTest helper invokeSetHidden to accept
a boolean hide parameter and pass it to the reflective setHidden invocation
instead of always using false. Add equivalent coverage for both hide values
across empty, status-bar, and navigation-bar inputs, ensuring the rewritten hide
branch in SystemBars.setHidden is exercised.
In `@android/CHANGELOG.md`:
- Around line 10-12: Change the 8.4.0 release heading in the changelog from
level-one (#) to level-two (##), matching adjacent release headings and keeping
the nested “Bug Fixes” heading hierarchy consistent.
In `@CHANGELOG.md`:
- Around line 6-52: Remove the manually added release entries from the root
CHANGELOG.md, including the 8.4.1 and 8.4.0 sections, and leave changelog
generation to the CI/CD release workflow.
- Around line 13-15: Change the 8.4.0 release heading in CHANGELOG.md from
level-one to level-two Markdown, matching the surrounding release headings and
preserving consistent hierarchy.
In `@cli/CHANGELOG.md`:
- Around line 13-15: Change the 8.4.0 release heading in cli/CHANGELOG.md from a
level-one heading to a level-two heading (`##`) so it matches the hierarchy used
by other release entries and correctly contains the existing `### Bug Fixes`
subsection.
In `@cli/src/declarations.ts`:
- Around line 809-821: Add `@since` JSDoc tags to the new symlink and
moduleAliases properties in PackageOptions, using the appropriate release
version consistent with nearby configuration fields and documentation
conventions.
In `@cli/src/ios/update.ts`:
- Around line 62-85: Move the getCapacitorPackageVersion call out of the
per-plugin callback and compute iosPlatformVersion once immediately before
Promise.all; then reuse that value inside validSPMPackages.map while preserving
the existing version comparison and update logic.
In `@cli/src/telemetry.ts`:
- Around line 85-90: Update the debug logging after the telemetry payload is
assembled so it does not dump sensitive CLI options or arguments. In the
`debug('metric payload: %O', data)` call, log only safe, non-sensitive telemetry
fields or create a redacted payload that removes credentials, tokens, paths, and
other option/argument values before logging.
In `@cli/src/util/spm.ts`:
- Around line 136-142: Update the path base used when computing `relPath` in the
package generation logic: use the directory containing `Package.swift`
(`nativeProjectDirAbs/CapApp-SPM`) rather than `nativeXcodeProjDirAbs` for the
non-symlink branch, and ensure the symlink path branch is relative to that same
manifest directory so generated `.package(path:)` entries resolve correctly.
- Around line 166-174: Guard moduleAliases generation in the plugin text
construction around pluginText by parsing experimental.ios.spm.swiftToolsVersion
and only emitting aliasText for Swift tools version 5.7 or newer; otherwise
reject the combination with a clear validation error. Ensure
packageOptions[plugin.id].moduleAliases cannot produce invalid Package.swift
output for older versions.
In `@core/package.json`:
- Around line 2-6: Update scripts/sync-peer-dependencies.mjs to replace all
`@capacitor-plus` package references with the renamed `@capacitor` scope, including
the corePkg lookup and Android/iOS package processing, so peer-dependency
synchronization targets `@capacitor/core`, `@capacitor/android`, and `@capacitor/ios`.
In `@ios/Capacitor/Capacitor/CAPBridgeViewController.swift`:
- Around line 133-137: Prevent duplicate cookie observer registration in
webViewConfiguration(for:). Before creating and assigning a new
CapacitorWKCookieObserver and cookie store, remove the existing cookieObserver
from cookieObserverStore when both are present, then clear or replace the stored
references only after removal. Preserve the existing add and assignment behavior
for the new observer.
- Around line 36-41: Update CAPBridgeViewController.deinit so
cookieObserverStore?.remove(cookieObserver) is dispatched to the main queue, or
otherwise guarantee main-thread isolation during teardown before interacting
with WebKit.
In `@ios/Capacitor/Capacitor/PluginConfig.swift`:
- Around line 33-39: Update getDouble to accept any numeric representation,
including Int and Double, instead of only casting directly to Double; inspect
the boxed value from KeyPath(configKey), handle it as NSNumber or explicitly
convert Int/other BinaryInteger values to Double, and return defaultValue only
for non-numeric or missing values. Verify JSObject/JSValue boxing behavior for
whole-number JSON literals and add or update tests covering integer and
floating-point config values.
In `@ios/CHANGELOG.md`:
- Around line 10-12: Change the 8.4.0 release heading in ios/CHANGELOG.md from
level-one (#) to level-two (##), matching the other release headings and
preserving the heading hierarchy before ### Features.
---
Outside diff comments:
In `@cli/src/tasks/run.ts`:
- Around line 118-127: Restore the Android manifest in the live-reload error
path within the catch block of the task runner. When options.liveReload is
enabled, call writeCordovaAndroidManifest with the appropriate Android
project/config arguments and false after reverting the Capacitor config,
ensuring failed runs restore AndroidManifest.xml before handling the error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bbe03d24-a36d-4662-b667-13ddaf092342
📒 Files selected for processing (30)
.github/workflows/test.ymlCHANGELOG.mdandroid/CHANGELOG.mdandroid/capacitor/src/main/java/com/getcapacitor/BridgeWebChromeClient.javaandroid/capacitor/src/main/java/com/getcapacitor/FileUtils.javaandroid/capacitor/src/main/java/com/getcapacitor/PluginConfig.javaandroid/capacitor/src/main/java/com/getcapacitor/WebViewLocalServer.javaandroid/capacitor/src/main/java/com/getcapacitor/cordova/MockCordovaWebViewImpl.javaandroid/capacitor/src/main/java/com/getcapacitor/plugin/CapacitorCookieManager.javaandroid/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.javaandroid/capacitor/src/main/java/com/getcapacitor/plugin/util/HttpRequestHandler.javaandroid/capacitor/src/main/java/com/getcapacitor/util/JSONUtils.javaandroid/capacitor/src/test/java/com/getcapacitor/plugin/SystemBarsTest.javaandroid/package.jsoncli/CHANGELOG.mdcli/package.jsoncli/src/declarations.tscli/src/ios/update.tscli/src/ipc.tscli/src/tasks/run.tscli/src/telemetry.tscli/src/util/spm.tscli/test/telemetry.spec.tscore/CHANGELOG.mdcore/package.jsonios/CHANGELOG.mdios/Capacitor/Capacitor/CAPBridgeViewController.swiftios/Capacitor/Capacitor/PluginConfig.swiftios/package.jsonlerna.json
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
| private WindowInsetsControllerCompat invokeSetHidden(String bar) throws Exception { | ||
| SystemBars plugin = new SystemBars(); | ||
| Bridge bridge = mock(Bridge.class); | ||
| AppCompatActivity activity = mock(AppCompatActivity.class); | ||
| Window window = mock(Window.class); | ||
| View decorView = mock(View.class); | ||
| WindowInsetsControllerCompat controller = mock(WindowInsetsControllerCompat.class); | ||
|
|
||
| when(bridge.getActivity()).thenReturn(activity); | ||
| when(activity.getWindow()).thenReturn(window); | ||
| when(window.getDecorView()).thenReturn(decorView); | ||
|
|
||
| plugin.setBridge(bridge); | ||
|
|
||
| try (MockedStatic<WindowCompat> windowCompat = mockStatic(WindowCompat.class)) { | ||
| windowCompat.when(() -> WindowCompat.getInsetsController(window, decorView)).thenReturn(controller); | ||
|
|
||
| Method setHidden = SystemBars.class.getDeclaredMethod("setHidden", boolean.class, String.class); | ||
| setHidden.setAccessible(true); | ||
| setHidden.invoke(plugin, false, bar); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Cover the rewritten hide branch.
The helper always invokes setHidden(false, bar) at Line 68, so the changed hide path in SystemBars.java is untested. Parameterize the helper with boolean hide and add equivalent tests for empty, status-bar, and navigation-bar inputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android/capacitor/src/test/java/com/getcapacitor/plugin/SystemBarsTest.java`
around lines 49 - 69, Update the SystemBarsTest helper invokeSetHidden to accept
a boolean hide parameter and pass it to the reflective setHidden invocation
instead of always using false. Add equivalent coverage for both hide values
across empty, status-bar, and navigation-bar inputs, ensuring the rewritten hide
branch in SystemBars.setHidden is exercised.
| # [8.4.0](https://github.com/ionic-team/capacitor/compare/8.3.4...8.4.0) (2026-06-02) | ||
|
|
||
| ### Bug Fixes |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a consistent release-heading level.
The 8.4.0 release is declared with #, while adjacent releases use ##; this also makes ### Bug Fixes skip a heading level. Change line 10 to ##.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 12-12: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android/CHANGELOG.md` around lines 10 - 12, Change the 8.4.0 release heading
in the changelog from level-one (#) to level-two (##), matching adjacent release
headings and keeping the nested “Bug Fixes” heading hierarchy consistent.
Source: Linters/SAST tools
| ## [8.4.1](https://github.com/ionic-team/capacitor/compare/8.4.0...8.4.1) (2026-06-19) | ||
|
|
||
| ### Bug Fixes | ||
|
|
||
| - **cli:** make SPM dependency patch work on prereleases ([#8508](https://github.com/ionic-team/capacitor/issues/8508)) ([6048e90](https://github.com/ionic-team/capacitor/commit/6048e90171afa0229a3c25b52a23c377c6bb804c)) | ||
| - **cli:** patch Capacitor SPM dependency version in plugins ([#8492](https://github.com/ionic-team/capacitor/issues/8492)) ([28bb2c6](https://github.com/ionic-team/capacitor/commit/28bb2c687069dfdd6aa7abc866004a1c6388d103)) | ||
|
|
||
| # [8.4.0](https://github.com/ionic-team/capacitor/compare/8.3.4...8.4.0) (2026-06-02) | ||
|
|
||
| ### Bug Fixes | ||
|
|
||
| - **android:** show only the requested system bar ([#8480](https://github.com/ionic-team/capacitor/issues/8480)) ([4c6c321](https://github.com/ionic-team/capacitor/commit/4c6c3219afb5223211e857457e46283c37eb9424)) | ||
| - **cli:** revert live reload config on failure ([#8485](https://github.com/ionic-team/capacitor/issues/8485)) ([1d031a4](https://github.com/ionic-team/capacitor/commit/1d031a4abec2c793079ba8897ad2e40c4cc6c7f9)) | ||
| - **SystemBars:** make `safe-area-inset-x` available on API <= 34 ([#8424](https://github.com/ionic-team/capacitor/issues/8424)) ([e456de0](https://github.com/ionic-team/capacitor/commit/e456de083e19644f484bec5a5359cb67960ac8bc)) | ||
| - **SystemBars:** respect `insetsHandling` disable ([#8481](https://github.com/ionic-team/capacitor/issues/8481)) ([d4ad7ff](https://github.com/ionic-team/capacitor/commit/d4ad7ffe39daf66e0cfc63af9028d5c05543bde7)) | ||
|
|
||
| ### Features | ||
|
|
||
| - add method getDouble to plugin config ([#7638](https://github.com/ionic-team/capacitor/issues/7638)) ([93c72de](https://github.com/ionic-team/capacitor/commit/93c72de40a2ec4c78b33659250cb08340083088e)) | ||
| - **cli:** add experimental packageOptions ([#8471](https://github.com/ionic-team/capacitor/issues/8471)) ([258867b](https://github.com/ionic-team/capacitor/commit/258867b7bf37b1837b99b02ec9638e5a6df08d97)) | ||
| - **cli:** capture ios_package_manager in telemetry ([#8482](https://github.com/ionic-team/capacitor/issues/8482)) ([b4b297a](https://github.com/ionic-team/capacitor/commit/b4b297a52f8732659662d5e5aaeff81c0f7d9835)) | ||
|
|
||
| ## [8.3.4](https://github.com/ionic-team/capacitor/compare/8.3.3...8.3.4) (2026-05-12) | ||
|
|
||
| **Note:** Version bump only for package capacitor | ||
|
|
||
| ## [8.3.3](https://github.com/ionic-team/capacitor/compare/8.3.2...8.3.3) (2026-05-08) | ||
|
|
||
| ### Bug Fixes | ||
|
|
||
| - **cli:** copy plugin files in CocoaPods projects ([#8467](https://github.com/ionic-team/capacitor/issues/8467)) ([b2d7719](https://github.com/ionic-team/capacitor/commit/b2d771926a180e60deea31992d7d4abcd5ca3bc7)) | ||
|
|
||
| ## [8.3.2](https://github.com/ionic-team/capacitor/compare/8.3.1...8.3.2) (2026-05-07) | ||
|
|
||
| ### Bug Fixes | ||
|
|
||
| - **cli:** add cSettings support for compiler flags in generated Package.swift ([#8448](https://github.com/ionic-team/capacitor/issues/8448)) ([0bd0676](https://github.com/ionic-team/capacitor/commit/0bd0676315c5fd77e50312dd7b5bf4990dcbd7d0)) | ||
| - **cli:** add system framework and weak framework support in SPM Package.swift ([#8447](https://github.com/ionic-team/capacitor/issues/8447)) ([3232f0f](https://github.com/ionic-team/capacitor/commit/3232f0fe1d9811b0b5c500e3dc05cb8a250177f8)) | ||
| - **cli:** correct Capacitor plugin SPM compat check ([#8440](https://github.com/ionic-team/capacitor/issues/8440)) ([e5ccc45](https://github.com/ionic-team/capacitor/commit/e5ccc451dda27d56bca824ed644bd20fe4d988cb)) | ||
| - **cli:** generate binaryTarget entries for custom xcframeworks in Package.swift ([#8445](https://github.com/ionic-team/capacitor/issues/8445)) ([1f7e33f](https://github.com/ionic-team/capacitor/commit/1f7e33fca43d183332ec19d22b0d75ef81d8cc6d)) | ||
| - **cli:** generate resource entries in Package.swift ([#8455](https://github.com/ionic-team/capacitor/issues/8455)) ([790bd27](https://github.com/ionic-team/capacitor/commit/790bd27123497111984227010c3162cec94a108e)) | ||
| - **cli:** handle Cordova plugins without iOS source files ([#8443](https://github.com/ionic-team/capacitor/issues/8443)) ([0da130e](https://github.com/ionic-team/capacitor/commit/0da130eb7a861bee4e2c35bc0aac53ba9c983fc3)) | ||
| - **cli:** link plugin dependencies in Package.swift ([#8457](https://github.com/ionic-team/capacitor/issues/8457)) ([b3c769e](https://github.com/ionic-team/capacitor/commit/b3c769e856c826b1174518877cf86ac7ce73bf09)) | ||
| - **ios:** support Cordova plugins with Package.swift ([#8438](https://github.com/ionic-team/capacitor/issues/8438)) ([139943b](https://github.com/ionic-team/capacitor/commit/139943b0c05fddb2d1ce2d6f468800fddf17b4cf)) | ||
| - **SystemBars:** avoid extra view padding on API <= 34 ([#8439](https://github.com/ionic-team/capacitor/issues/8439)) ([5b135a7](https://github.com/ionic-team/capacitor/commit/5b135a70217be560e7176c8d5b514cc92ed3e4e4)) | ||
|
|
||
| ## [8.3.1](https://github.com/ionic-team/capacitor/compare/8.3.0...8.3.1) (2026-04-16) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not manually edit the root changelog.
CHANGELOG.md is managed automatically by CI/CD. Remove this manual release-note change and let the release workflow regenerate it.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 15-15: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` around lines 6 - 52, Remove the manually added release entries
from the root CHANGELOG.md, including the 8.4.1 and 8.4.0 sections, and leave
changelog generation to the CI/CD release workflow.
Source: Coding guidelines
| # [8.4.0](https://github.com/ionic-team/capacitor/compare/8.3.4...8.4.0) (2026-06-02) | ||
|
|
||
| ### Bug Fixes |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep release headings at the same level.
8.4.0 uses # while the surrounding release headings use ##, causing the heading hierarchy to jump and triggering MD001. Change line 13 to ##.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 15-15: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` around lines 13 - 15, Change the 8.4.0 release heading in
CHANGELOG.md from level-one to level-two Markdown, matching the surrounding
release headings and preserving consistent hierarchy.
Source: Linters/SAST tools
| # [8.4.0](https://github.com/ionic-team/capacitor/compare/8.3.4...8.4.0) (2026-06-02) | ||
|
|
||
| ### Bug Fixes |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a consistent release-heading level.
The 8.4.0 release uses # instead of ##, so the following ### Bug Fixes heading violates the expected hierarchy. Change line 13 to ##.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 15-15: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/CHANGELOG.md` around lines 13 - 15, Change the 8.4.0 release heading in
cli/CHANGELOG.md from a level-one heading to a level-two heading (`##`) so it
matches the hierarchy used by other release entries and correctly contains the
existing `### Bug Fixes` subsection.
Source: Linters/SAST tools
| "name": "@capacitor/core", | ||
| "version": "8.4.1", | ||
| "description": "Capacitor: Cross-platform apps with JavaScript and the web", | ||
| "homepage": "https://capacitorjs.com", | ||
| "author": "Ionic Team <hi@ionic.io> (https://ionic.io)", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Synchronize the peer-dependency script with the new package scope.
scripts/sync-peer-dependencies.mjs still searches for @capacitor-plus/core and only processes @capacitor-plus/android/@capacitor-plus/ios. After this rename, corePkg can be undefined and release synchronization can fail or leave the Android/iOS peer dependencies unmanaged. Update that script to use the new @capacitor/* names before merging.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/package.json` around lines 2 - 6, Update
scripts/sync-peer-dependencies.mjs to replace all `@capacitor-plus` package
references with the renamed `@capacitor` scope, including the corePkg lookup and
Android/iOS package processing, so peer-dependency synchronization targets
`@capacitor/core`, `@capacitor/android`, and `@capacitor/ios`.
| deinit { | ||
| if let cookieObserver = cookieObserver { | ||
| cookieObserverStore?.remove(cookieObserver) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CAPBridgeViewController.swift =="
ast-grep outline ios/Capacitor/Capacitor/CAPBridgeViewController.swift --view expanded || true
echo
echo "== Relevant lines =="
cat -n ios/Capacitor/Capacitor/CAPBridgeViewController.swift | sed -n '1,220p'
echo
echo "== Search for cookieObserver symbols =="
rg -n "cookieObserver|cookieObserverStore|WKHTTPCookieStore|add\\(|remove\\(" ios/Capacitor/Capacitor -S || trueRepository: Cap-go/capacitor-plus
Length of output: 14548
🌐 Web query:
Apple WKHTTPCookieStore add remove main thread documentation
💡 Result:
WKHTTPCookieStore methods for adding and removing cookies, such as setCookie(:completionHandler:) and deleteCookie(:completionHandler:), are asynchronous operations [1][2][3]. While Apple's official documentation does not explicitly mandate that these calls must originate from the main thread [4][3], developers frequently report issues—such as failed completion handler execution or synchronization conflicts—when these operations are not handled carefully [1][5]. Recent technical discussions and reports suggest that invoking these methods from the main thread can help ensure stability and avoid issues with thread-safety and internal WebKit assertions [6]. Notably, there have been reports in modern Swift development where async methods on WKHTTPCookieStore do not execute on the main thread as expected, even when annotated with @MainActor, which can trigger assertions [6]. To ensure reliable cookie management: 1. Operations are inherently asynchronous and use completion handlers [1][3]. 2. For UI-related updates or to maintain synchronization with WKWebView, developers often dispatch these calls to the main thread [1][7]. 3. Synchronization issues are often linked to the use of different WKProcessPool instances; using a consistent, shared WKProcessPool for your web content is a recommended best practice to ensure the cookie store functions as expected across different web views [1][7][5]. Always test your specific implementation, as the behavior of completion handlers can depend on the lifecycle of the underlying WebsiteDataStore and the WKWebView process [1][5].
Citations:
- 1: https://dobrean.ro/2019/05/19/wkwebview-and-wkcookiestore-in-ios-11/
- 2: https://stackoverflow.com/questions/63401390/wkhttpcookiestore-httpcookiestorage-synchronization-class
- 3: https://developer.apple.com/documentation/webkit/wkhttpcookiestore/setcookie(_:completionhandler:)
- 4: https://developer.apple.com/documentation/webkit/wkhttpcookiestore
- 5: https://developer.apple.com/forums/message/333176
- 6: Async method calls on WKHTTPCookieStore run on non-main thread despite @MainActor annotation swiftlang/swift#80398
- 7: https://developer.apple.com/forums/thread/97194
🌐 Web query:
Swift deinit may run on background thread UIViewController deallocation thread
💡 Result:
In UIKit, UIViewController objects have historically maintained an implementation that attempts to ensure deallocation occurs on the main thread, as UIKit objects are generally not thread-safe and deallocation often involves cleanup of view hierarchies that must occur on the main thread [1][2][3]. However, this behavior is not a guaranteed absolute in all scenarios; if the last strong reference to a UIViewController is released on a background thread, the deallocation process—including the execution of the deinit method—can trigger on that background thread [4][3]. Because UIKit relies on thread-unsafe components, executing deinit on a background thread for a UIViewController can lead to crashes or undefined behavior if that deinit performs operations requiring main-thread access (e.g., modifying view properties or interacting with other UI components) [2][3]. With recent advancements in Swift concurrency, specifically SE-0371 (Isolated Synchronous Deinit), Swift provides improved tools for managing these issues [5][6]. Developers can now use isolated deinits for actor-isolated or global-actor-isolated classes to ensure the deinitialization body runs on the correct executor [5][6]. For types like UIViewController, relying on historical deallocation behavior is discouraged; developers should ensure that they do not hold references to UIViewController instances in blocks or tasks that might outlive the object on a background thread [4][3]. If necessary, modern Swift concurrency features allow for more robust control over the environment in which deinitialization occurs [5][6].
Citations:
- 1: https://exchangetuts.com/why-is-uiviewcontroller-deallocated-on-the-main-thread-1641185367702320
- 2: https://www.objc.io/issues/2-concurrency/thread-safe-class-design
- 3: https://www.exchangetuts.com/dealloc-on-background-thread-1641120123815936
- 4: https://stackoverflow.com/questions/5527930/blocks-and-viewcontroller-thread-safety
- 5: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0371-isolated-synchronous-deinit.md
- 6: https://forums.swift.org/t/pitch-2-se-0371-isolated-async-deinit/64836
Dispatch cookie-observer cleanup on the main queue
deinit can run off-main, so cookieObserverStore?.remove(cookieObserver) may hit WebKit from a background thread. Move this cleanup to the main queue or otherwise ensure the controller is main-isolated during teardown.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/Capacitor/Capacitor/CAPBridgeViewController.swift` around lines 36 - 41,
Update CAPBridgeViewController.deinit so
cookieObserverStore?.remove(cookieObserver) is dispatched to the main queue, or
otherwise guarantee main-thread isolation during teardown before interacting
with WebKit.
| let cookieObserver = CapacitorWKCookieObserver() | ||
| let cookieStore = webViewConfiguration.websiteDataStore.httpCookieStore | ||
| cookieStore.add(cookieObserver) | ||
| self.cookieObserver = cookieObserver | ||
| self.cookieObserverStore = cookieStore |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Guard against overwriting a still-registered observer if webViewConfiguration(for:) runs more than once.
Each call unconditionally overwrites cookieObserver/cookieObserverStore without first removing any prior registration. Since WKHTTPCookieStore does not retain observers, an overwritten (and now unreferenced) CapacitorWKCookieObserver from a prior call would simply be deallocated while still nominally "added" to its store, leaving a stale/dangling entry rather than a clean removal. This method is open and documented as safe to call via super, so a subclass or an unexpected second invocation of loadView()/webViewConfiguration(for:) could trigger this path.
🛡️ Proposed defensive fix
open func webViewConfiguration(for instanceConfiguration: InstanceConfiguration) -> WKWebViewConfiguration {
let webViewConfiguration = WKWebViewConfiguration()
+ if let existingObserver = cookieObserver {
+ cookieObserverStore?.remove(existingObserver)
+ }
let cookieObserver = CapacitorWKCookieObserver()
let cookieStore = webViewConfiguration.websiteDataStore.httpCookieStore
cookieStore.add(cookieObserver)
self.cookieObserver = cookieObserver
self.cookieObserverStore = cookieStore📝 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.
| let cookieObserver = CapacitorWKCookieObserver() | |
| let cookieStore = webViewConfiguration.websiteDataStore.httpCookieStore | |
| cookieStore.add(cookieObserver) | |
| self.cookieObserver = cookieObserver | |
| self.cookieObserverStore = cookieStore | |
| let webViewConfiguration = WKWebViewConfiguration() | |
| if let existingObserver = cookieObserver { | |
| cookieObserverStore?.remove(existingObserver) | |
| } | |
| let cookieObserver = CapacitorWKCookieObserver() | |
| let cookieStore = webViewConfiguration.websiteDataStore.httpCookieStore | |
| cookieStore.add(cookieObserver) | |
| self.cookieObserver = cookieObserver | |
| self.cookieObserverStore = cookieStore |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/Capacitor/Capacitor/CAPBridgeViewController.swift` around lines 133 -
137, Prevent duplicate cookie observer registration in
webViewConfiguration(for:). Before creating and assigning a new
CapacitorWKCookieObserver and cookie store, remove the existing cookieObserver
from cookieObserverStore when both are present, then clear or replace the stored
references only after removal. Preserve the existing add and assignment behavior
for the new observer.
| @objc public func getDouble(_ configKey: String, _ defaultValue: Double) -> Double { | ||
| if let val = (self.config)[keyPath: KeyPath(configKey)] as? Double { | ||
| return val | ||
| } | ||
| return defaultValue | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
getDouble may silently fall back to default for whole-number config values.
If the config value is authored as a JSON integer (e.g. "timeout": 30) rather than a float literal, the underlying boxed value is likely an Int, and as? Double will fail to cast, causing a silent fallback to defaultValue. The Android counterpart (JSONUtils.getDouble → JSONObject.getDouble) coerces any Number type via .doubleValue(), so this iOS accessor's behavior can diverge from Android's for the same config value.
♻️ Proposed fix to accept both Int and Double representations
`@objc` public func getDouble(_ configKey: String, _ defaultValue: Double) -> Double {
- if let val = (self.config)[keyPath: KeyPath(configKey)] as? Double {
- return val
+ if let val = (self.config)[keyPath: KeyPath(configKey)] as? Double {
+ return val
+ }
+ if let intVal = (self.config)[keyPath: KeyPath(configKey)] as? Int {
+ return Double(intVal)
}
return defaultValue
}Please confirm how JSObject/JSValue box whole-number JSON literals on iOS to verify whether this fallback is actually reachable in practice.
📝 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.
| @objc public func getDouble(_ configKey: String, _ defaultValue: Double) -> Double { | |
| if let val = (self.config)[keyPath: KeyPath(configKey)] as? Double { | |
| return val | |
| } | |
| return defaultValue | |
| } | |
| `@objc` public func getDouble(_ configKey: String, _ defaultValue: Double) -> Double { | |
| if let val = (self.config)[keyPath: KeyPath(configKey)] as? Double { | |
| return val | |
| } | |
| if let intVal = (self.config)[keyPath: KeyPath(configKey)] as? Int { | |
| return Double(intVal) | |
| } | |
| return defaultValue | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/Capacitor/Capacitor/PluginConfig.swift` around lines 33 - 39, Update
getDouble to accept any numeric representation, including Int and Double,
instead of only casting directly to Double; inspect the boxed value from
KeyPath(configKey), handle it as NSNumber or explicitly convert Int/other
BinaryInteger values to Double, and return defaultValue only for non-numeric or
missing values. Verify JSObject/JSValue boxing behavior for whole-number JSON
literals and add or update tests covering integer and floating-point config
values.
| # [8.4.0](https://github.com/ionic-team/capacitor/compare/8.3.4...8.4.0) (2026-06-02) | ||
|
|
||
| ### Features |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a consistent release-heading level.
The 8.4.0 release uses # instead of ##, causing the following ### Features heading to skip a level. Change line 10 to ##.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 12-12: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/CHANGELOG.md` around lines 10 - 12, Change the 8.4.0 release heading in
ios/CHANGELOG.md from level-one (#) to level-two (##), matching the other
release headings and preserving the heading hierarchy before ### Features.
Source: Linters/SAST tools
There was a problem hiding this comment.
Risk: high. Not approving: this upstream sync has broad iOS/Android/CLI changes from merge-conflict resolution, reverts package names from @capacitor-plus/* to @capacitor/*, and Cursor Bugbot was not present. Human review is required; assigned jcesarmobile and carlpoole.
Sent by Cursor Approval Agent: Pull Request Approver
8 participants
Merge Conflict Resolution Required
The sync of upstream PR ionic-team#8520 from @Cybertron01Z encountered merge conflicts.
Original PR: ionic-team#8520
What happened
Synced from upstream by Capacitor+ Bot
Summary by CodeRabbit
New Features
Bug Fixes
Documentation