From 49d20fc31183c649fab1d7432bf237476a809073 Mon Sep 17 00:00:00 2001 From: Kirill Zyusko Date: Wed, 29 Jan 2025 17:54:27 +0100 Subject: [PATCH] fix: crash in `KeyboardMovementObserver` due to incorrect KVO removal (#787) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📜 Description Fixed a crash in `KeyboardMovementObserver` due to incorrect KVO removal. ## 💡 Motivation and Context Similar to https://github.com/kirillzyusko/react-native-keyboard-controller/pull/768 Instead of using plain KVO we are using block-scoped KVO, mainly to prevent random crashes, when we try to remove a KVO from a view that doesn't have it. I can not 100% gurantee, that crash will not happen again, because a crash was very random and we can not reproduce it ourself (we just see it in Sentry) - so basically I can not test it. But I'm pretty confident, that a new approach will fix the issue, since we will not accidentally remove KVO where it doesn't exist (because of block-scoped approach) 🤞 Also new approach reduces code duplication (such as `if keyPath == "center"`) and reduces a number of code lines 👀 Fixes https://github.com/kirillzyusko/react-native-keyboard-controller/issues/152#issuecomment-2620518076 ## 📢 Changelog ### iOS - use block-scoped KVO in `KeyboardMovementObserver`; ## 🤔 How Has This Been Tested? Tested manually on iPhone 15 Pro (iOS 17.5). ## 📸 Screenshots (if appropriate): https://github.com/user-attachments/assets/acb009a1-8c25-44c5-93ff-cf1ad0a48dc7 ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed --- ios/observers/KeyboardMovementObserver.swift | 96 ++++++++------------ 1 file changed, 40 insertions(+), 56 deletions(-) diff --git a/ios/observers/KeyboardMovementObserver.swift b/ios/observers/KeyboardMovementObserver.swift index 38821a9629..d1a08a92f8 100644 --- a/ios/observers/KeyboardMovementObserver.swift +++ b/ios/observers/KeyboardMovementObserver.swift @@ -33,7 +33,7 @@ public class KeyboardMovementObserver: NSObject { private var _windowsCount: Int = 0 private var prevKeyboardPosition = 0.0 private var displayLink: CADisplayLink? - private var hasKVObserver = false + private var interactiveKeyboardObserver: NSKeyValueObservation? private var isMounted = false // state variables private var _keyboardHeight: CGFloat = 0.0 @@ -93,73 +93,57 @@ public class KeyboardMovementObserver: NSObject { } private func setupKVObserver() { - if hasKVObserver { - return - } + guard interactiveKeyboardObserver == nil, let view = keyboardView else { return } + + interactiveKeyboardObserver = view.observe(\.center, options: .new) { [weak self] _, change in + guard let self = self, let changeValue = change.newValue else { return } - if keyboardView != nil { - hasKVObserver = true - keyboardView?.addObserver(self, forKeyPath: "center", options: .new, context: nil) + self.keyboardDidMoveInteractively(changeValue: changeValue) } } private func removeKVObserver() { - if !hasKVObserver { + interactiveKeyboardObserver?.invalidate() + interactiveKeyboardObserver = nil + } + + private func keyboardDidMoveInteractively(changeValue: CGPoint) { + // if we are currently animating keyboard -> we need to ignore values from KVO + if displayLink != nil { + return + } + // if keyboard height is not equal to its bounds - we can ignore + // values, since they'll be invalid and will cause UI jumps + if floor(keyboardView?.bounds.size.height ?? 0) != floor(_keyboardHeight) { return } - hasKVObserver = false - _keyboardView?.removeObserver(self, forKeyPath: "center", context: nil) - } + let keyboardFrameY = changeValue.y + let keyboardWindowH = keyboardView?.window?.bounds.size.height ?? 0 + let keyboardPosition = keyboardWindowH - keyboardFrameY - // swiftlint:disable:next block_based_kvo - @objc override public func observeValue( - forKeyPath keyPath: String?, - of object: Any?, - change: [NSKeyValueChangeKey: Any]?, - context _: UnsafeMutableRawPointer? - ) { - if keyPath == "center", object as? NSObject == _keyboardView { - // if we are currently animating keyboard -> we need to ignore values from KVO - if displayLink != nil { - return - } - // if keyboard height is not equal to its bounds - we can ignore - // values, since they'll be invalid and will cause UI jumps - if floor(keyboardView?.bounds.size.height ?? 0) != floor(_keyboardHeight) { - return - } + let position = CGFloat.interpolate( + inputRange: [_keyboardHeight / 2, -_keyboardHeight / 2], + outputRange: [_keyboardHeight, 0], + currentValue: keyboardPosition + ) - KeyboardAreaExtender.shared.offset - guard let changeValue = change?[.newKey] as? NSValue else { - return - } - let keyboardFrameY = changeValue.cgPointValue.y - let keyboardWindowH = keyboardView?.window?.bounds.size.height ?? 0 - let keyboardPosition = keyboardWindowH - keyboardFrameY - - let position = CGFloat.interpolate( - inputRange: [_keyboardHeight / 2, -_keyboardHeight / 2], - outputRange: [_keyboardHeight, 0], - currentValue: keyboardPosition - ) - KeyboardAreaExtender.shared.offset - - if position == 0 { - // it will be triggered before `keyboardWillDisappear` and - // we don't need to trigger `onInteractive` handler for that - // since it will be handled in `keyboardWillDisappear` function - return - } + if position == 0 { + // it will be triggered before `keyboardWillDisappear` and + // we don't need to trigger `onInteractive` handler for that + // since it will be handled in `keyboardWillDisappear` function + return + } - prevKeyboardPosition = position + prevKeyboardPosition = position - onEvent( - "onKeyboardMoveInteractive", - position as NSNumber, - position / CGFloat(keyboardHeight) as NSNumber, - -1, - tag - ) - } + onEvent( + "onKeyboardMoveInteractive", + position as NSNumber, + position / CGFloat(keyboardHeight) as NSNumber, + -1, + tag + ) } @objc public func unmount() {