Fix(ios) Only add orientation observer when rotateWhenOrientationChanged is true #287
Conversation
📝 WalkthroughWalkthroughDevice orientation notification handling in the Camera Preview Plugin is now conditional on the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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
🤖 Fix all issues with AI agents
In @ios/Sources/CapgoCameraPreviewPlugin/Plugin.swift:
- Around line 781-790: The code calls
UIDevice.current.beginGeneratingDeviceOrientationNotifications() only when
rotateWhenOrientationChanged is true but always calls
endGeneratingDeviceOrientationNotifications() in stop()/force cleanup, risking
an unbalanced decrement; add a boolean property (e.g.
didStartOrientationNotifications or isGeneratingOrientationNotifications) to the
Plugin class, set it to true inside the DispatchQueue.main.async block when
beginGeneratingDeviceOrientationNotifications() is invoked (or when
rotateWhenOrientationChanged is true), and in stop(), force cleanup and any
other teardown paths only call
UIDevice.current.endGeneratingDeviceOrientationNotifications() if that boolean
is true and then reset it to false; reference rotateWhenOrientationChanged,
handleOrientationChange, completeStartCamera(call:), start(), stop(), and the
force cleanup paths when making the changes.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ios/Sources/CapgoCameraPreviewPlugin/Plugin.swift
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T15:37:50.891Z
Learnt from: Dalanir
Repo: Cap-go/capacitor-camera-preview PR: 217
File: ios/Sources/CapgoCameraPreviewPlugin/CameraController.swift:1242-1246
Timestamp: 2025-08-29T15:37:50.891Z
Learning: In the iOS CameraController.swift handleTap method, the team wants to maintain the current behavior that forces exposureMode to .autoExpose and resets exposureTargetBias to 0.0 when tapping to focus, even though it overrides the user's current exposure mode setting.
Applied to files:
ios/Sources/CapgoCameraPreviewPlugin/Plugin.swift
| DispatchQueue.main.async { | ||
| UIDevice.current.beginGeneratingDeviceOrientationNotifications() | ||
| NotificationCenter.default.addObserver(self, | ||
| selector: #selector(self.handleOrientationChange), | ||
| name: UIDevice.orientationDidChangeNotification, | ||
| object: nil) | ||
| if self.rotateWhenOrientationChanged == true { | ||
| UIDevice.current.beginGeneratingDeviceOrientationNotifications() | ||
| NotificationCenter.default.addObserver(self, | ||
| selector: #selector(self.handleOrientationChange), | ||
| name: UIDevice.orientationDidChangeNotification, | ||
| object: nil) | ||
| } | ||
| self.completeStartCamera(call: call) | ||
| } |
There was a problem hiding this comment.
Balance beginGeneratingDeviceOrientationNotifications() / endGeneratingDeviceOrientationNotifications() now that start() is conditional.
Gating the observer fixes the reported first-start repositioning bug, but stop() (and the force cleanup) still call UIDevice.current.endGeneratingDeviceOrientationNotifications() unconditionally, which can become an unbalanced decrement and potentially interfere with other orientation-notification users in the app process.
Recommend tracking whether this plugin started generation and only ending when it did.
Proposed fix (track and conditionally end)
public class CameraPreview: CAPPlugin, CAPBridgedPlugin, CLLocationManagerDelegate {
+ private var isGeneratingDeviceOrientationNotifications = false
private let pluginVersion: String = "8.0.6"
public let identifier = "CameraPreviewPlugin"
public let jsName = "CameraPreview"
@@
DispatchQueue.main.async {
if self.rotateWhenOrientationChanged == true {
UIDevice.current.beginGeneratingDeviceOrientationNotifications()
+ self.isGeneratingDeviceOrientationNotifications = true
NotificationCenter.default.addObserver(self,
selector: #selector(self.handleOrientationChange),
name: UIDevice.orientationDidChangeNotification,
object: nil)
}
self.completeStartCamera(call: call)
}
@@
// Remove notification observers regardless
NotificationCenter.default.removeObserver(self)
NotificationCenter.default.removeObserver(self, name: UIDevice.orientationDidChangeNotification, object: nil)
- UIDevice.current.endGeneratingDeviceOrientationNotifications()
+ if self.isGeneratingDeviceOrientationNotifications {
+ UIDevice.current.endGeneratingDeviceOrientationNotifications()
+ self.isGeneratingDeviceOrientationNotifications = false
+ } // In the `force` cleanup block inside start(_:) as well:
-NotificationCenter.default.removeObserver(self, name: UIDevice.orientationDidChangeNotification, object: nil)
-UIDevice.current.endGeneratingDeviceOrientationNotifications()
+NotificationCenter.default.removeObserver(self, name: UIDevice.orientationDidChangeNotification, object: nil)
+if self.isGeneratingDeviceOrientationNotifications {
+ UIDevice.current.endGeneratingDeviceOrientationNotifications()
+ self.isGeneratingDeviceOrientationNotifications = false
+}🤖 Prompt for AI Agents
In @ios/Sources/CapgoCameraPreviewPlugin/Plugin.swift around lines 781 - 790,
The code calls UIDevice.current.beginGeneratingDeviceOrientationNotifications()
only when rotateWhenOrientationChanged is true but always calls
endGeneratingDeviceOrientationNotifications() in stop()/force cleanup, risking
an unbalanced decrement; add a boolean property (e.g.
didStartOrientationNotifications or isGeneratingOrientationNotifications) to the
Plugin class, set it to true inside the DispatchQueue.main.async block when
beginGeneratingDeviceOrientationNotifications() is invoked (or when
rotateWhenOrientationChanged is true), and in stop(), force cleanup and any
other teardown paths only call
UIDevice.current.endGeneratingDeviceOrientationNotifications() if that boolean
is true and then reset it to false; reference rotateWhenOrientationChanged,
handleOrientationChange, completeStartCamera(call:), start(), stop(), and the
force cleanup paths when making the changes.
There was a problem hiding this comment.
@riderx I apologize if this obvious, but I'm pretty new to contributing on these cap plugins. Do you want me to do what the coderabbitai is suggesting? or is that something you/maintainers decide?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
2 participants
Description
Fixes an issue where the camera preview repositions on first start even when
rotateWhenOrientationChangedis set tofalse.Problem
The orientation change observer was being added unconditionally in the
start()method. When added, the iOS notification system fires immediately if the device already has an orientation. On first camera start,lastOrientationisnil, so the early-return check inhandleOrientationChange()fails, causing the camera to reposition viarawSetAspectRatio().On subsequent starts,
lastOrientationis set from the previous session, so the early-return prevents repositioning - making this a first-start-only bug.Solution
Wrap the orientation observer registration in a conditional check:
This matches the existing pattern at line ~852 where rotation observers are already conditionally added.
Testing
Tested on iPhone 14 Pro with:
rotateWhenOrientationChanged: falsey: 80(explicit positioning)Before: Camera repositions to y=0 on first start
After: Camera stays at y=80 on first and subsequent starts
Changes
start()method to conditionally add orientation observerstop()method to conditionally remove orientation observerRelated
This only affects iOS. Android uses
OrientationEventListenerwhich is event-driven and doesn't fire on initial registration.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.