chore: sync upstream PR #5706 - feat: Add referrerUrl#2
Conversation
|
Warning Rate limit exceeded@riderx has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThese changes add referrer URL tracking for universal links in the iOS Capacitor framework. A new public Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ios/Capacitor/Capacitor/CAPApplicationDelegateProxy.swift(2 hunks)ios/Capacitor/Capacitor/CAPBridge.swift(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ios/Capacitor/Capacitor/CAPApplicationDelegateProxy.swift (1)
ios/Capacitor/Capacitor/CAPNotifications.swift (1)
name(52-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-android
- GitHub Check: test-core
🔇 Additional comments (3)
ios/Capacitor/Capacitor/CAPApplicationDelegateProxy.swift (2)
28-28: I'll help you verify and rewrite this review comment. Let me start by examining the file and checking the iOS deployment target configuration.
<function_calls>
#!/bin/bashFirst, let's examine the file and context around line 28
cat -n ios/Capacitor/Capacitor/CAPApplicationDelegateProxy.swift | head -40
</function_calls>
#!/bin/bashCheck the iOS deployment target in pbxproj files
fd -e pbxproj | head -5 | xargs -I {} grep -n "IPHONEOS_DEPLOYMENT_TARGET" {} | head -20
</function_calls>
NSUserActivity referrerURL iOS API availability minimum version
</function_calls>
29-32: Notification payload correctly handles optionalreferrerUrl; external consumers must verify proper nil handling on their end.The implementation properly handles the optional
referrerUrl:
referrerUrlproperty is correctly typed as optional (URL?)- It's assigned from
userActivity.referrerURL, which is already optional- The notification payload includes it as-is, allowing nil values
CAPBridge.getReferrerUrl()exposes it as optional to callersExternal plugins and the JavaScript layer consuming
.capacitorOpenUniversalLinknotifications must handle the nil case when accessing thereferrerUrlfield from the notification payload.ios/Capacitor/Capacitor/CAPBridge.swift (1)
14-16: NSUserActivity.referrerURL has been available since iOS 9, not iOS 11. The project targets iOS 15.0, which is well above the minimum requirement.Based on Apple's official documentation, the
NSUserActivity.referrerURLproperty was introduced in iOS 9 as part of the App Search and NSUserActivity APIs, not iOS 11 as stated in the review comment. Since this Capacitor project has a deployment target of iOS 15.0 (confirmed inios/Capacitor/Capacitor.xcodeproj/project.pbxproj), there are no compatibility concerns. The implementation is safe to use.
| public static let shared = ApplicationDelegateProxy() | ||
|
|
||
| public private(set) var lastURL: URL? | ||
| public private(set) var referrerUrl: URL? |
There was a problem hiding this comment.
Consider when referrerUrl should be cleared.
The referrerUrl property is set during universal link handling but is never cleared. Unlike lastURL (which is updated on every URL open), referrerUrl may persist across multiple app launches or URL opens, potentially leading to stale data.
Consider whether referrerUrl should be:
- Cleared when handling regular URL opens (in
application(_:open:options:)on line 10) - Cleared when handling universal links that have no referrer
- Left as-is if the intent is to preserve the most recent referrer
Please clarify the intended lifecycle behavior.
🤖 Prompt for AI Agents
In ios/Capacitor/Capacitor/CAPApplicationDelegateProxy.swift around line 8,
referrerUrl is set during universal link handling but never cleared, which can
leave stale data across launches; modify handling so referrerUrl is cleared when
a non-universal URL is opened (application(_:open:options:)), and also
explicitly clear it when a universal link is processed that contains no referrer
(or when processing fails); implement these clears alongside the existing
updates to lastURL and add a short comment documenting the intended lifecycle
(referrer only valid immediately after a universal link is handled).
riderx
force-pushed
the
sync/upstream-pr-5706
branch
from
e088ca9 to
35f0d8d
Compare
|
Thanks for your PR. Security-related changes should be coordinated through a private advisory before opening a public PR.\n\nPlease open a private advisory here:\n- https://capgo.app/fr/security/\n- https://github.com/Cap-go/capgo/security/advisories/new\n\nI am closing this PR for now. |
|
Thanks for the submission. |
|
This PR appears to describe a security-related issue publicly. For security changes, please use a private advisory before opening a public PR. See https://capgo.app/fr/security/ and https://github.com/Cap-go/capgo/security/advisories/new. |
|
Thanks for your submission. Security-sensitive public PRs should be disclosed via private advisory first. |
2 participants
Upstream PR Sync
This PR syncs changes from an external contributor's PR on the official Capacitor repository.
Original PR
Automation
Synced from upstream by Capacitor+ Bot
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.