Skip to content
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

fix: NSRangeException in FocusedInputObserver #768

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

kirillzyusko
Copy link
Owner

@kirillzyusko kirillzyusko commented Jan 20, 2025

📜 Description

Potential fix for NSRangeException in FocusedInputObserver.

💡 Motivation and Context

We don't have a reproduction example, so this fix is kind of blind. To fix this problem I decided to use block-scoped KVO. It has several advantages:

  • we always assure that we can remove only a valid listener (and will not try to remove unexisting listener);
  • we don't ignore swiftlint rules;
  • we reduce number of lines in the code;
  • we simplify KVO management and don't introduce additional boolean fields for its management.

So I think these changes should fix a problem 🤞

Closes #767

📢 Changelog

iOS

  • use scope-based KVO instead of old style with observeValue class member;

🤔 How Has This Been Tested?

Tested on CI to confirm that functionality of the library is not broken.

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

@kirillzyusko kirillzyusko self-assigned this Jan 20, 2025
@kirillzyusko kirillzyusko added 🍎 iOS iOS specific 🎯 crash Library triggers a crash of the app focused input 📝 Anything about focused input functionality labels Jan 20, 2025
Copy link
Contributor

📊 Package size report

Current size Target Size Difference
167699 bytes 167655 bytes 44 bytes 📈

@kirillzyusko kirillzyusko marked this pull request as ready for review January 21, 2025 09:36
@kirillzyusko kirillzyusko merged commit dd31184 into main Jan 21, 2025
15 checks passed
@kirillzyusko kirillzyusko deleted the fix/nsrange-exception branch January 21, 2025 10:01
kirillzyusko added a commit that referenced this pull request Jan 29, 2025
…#787)

## 📜 Description

Fixed a crash in `KeyboardMovementObserver` due to incorrect KVO
removal.

## 💡 Motivation and Context

Similar to
#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
#152 (comment)

## 📢 Changelog

<!-- High level overview of important changes -->
<!-- For example: fixed status bar manipulation; added new types
declarations; -->
<!-- If your changes don't affect one of platform/language below - then
remove this platform/language -->

### 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎯 crash Library triggers a crash of the app focused input 📝 Anything about focused input functionality 🍎 iOS iOS specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] Cannot remove an observer <FocusedInputObserver 0x301fce800> for the key path "center"
1 participant