Skip to content

iOS crash when native alert is shown while keyboard is visible #175

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

Closed
owinter86 opened this issue Jun 16, 2023 · 15 comments · Fixed by #178
Closed

iOS crash when native alert is shown while keyboard is visible #175

owinter86 opened this issue Jun 16, 2023 · 15 comments · Fixed by #178
Assignees
Labels
🎯 crash Library triggers a crash of the app 🍎 iOS iOS specific

Comments

@owinter86
Copy link

Describe the bug
iOS crashes when using a native alert while the keyboard is shown, I can confirm this only happens if the app is wrapped in the KeyboardProvider, removing the Keyboard provider from the snippet below will avoid the crash on ios.

Code snippet

import { Alert, Button, TextInput, View } from "react-native";
import { KeyboardProvider } from "react-native-keyboard-controller";

export function App() {
  return (
    <KeyboardProvider>
      <TestView />
    </KeyboardProvider>
  );
}

function TestView() {
  return (
    <View style={{ flex: 1, alignItems: "center", justifyContent: "center" }}>
      <TextInput placeholder="PLACHOLDER TEXT" />
      <Button
        onPress={() => {
          Alert.alert("Hello");
        }}
        title="Show Alert"
      />
    </View>
  );
}

Repo for reproducing
No repo provided as the code sample is simple enough to paste into a running env without having to checkout and build another repo.

To Reproduce
Steps to reproduce the behavior:

  1. Select placeholder text to expose keyboard
  2. press show alert button
  3. see crash

Expected behavior
Alerts should not crash the app when the keyboard is shown

Screenshots
If applicable, add screenshots to help explain your problem.

Smartphone (please complete the following information):

  • Desktop OS: MacOS 13.4 (22F66)
  • Device: iPhone 13 pro
  • OS: 16.5
  • RN version: 0.71.8 - Expo SDK 48.0.18
  • RN architecture: old arch
  • JS engine: hermes
  • Library version: 1.5.6
@kirillzyusko
Copy link
Owner

Thanks @owinter86!

I'll have a look on that!

@kirillzyusko
Copy link
Owner

kirillzyusko commented Jun 16, 2023

@owinter86 it seems like I can not reproduce it 😔

Simulator.Screen.Recording.-.iPhone.14.-.2023-06-16.at.09.00.29.mp4

I've tried to reproduce it on simulator. Should I try to use a real device?

P. S. I also tested app on iOS 16.2. Maybe 16.5 affects somehow? 🤔

@owinter86
Copy link
Author

owinter86 commented Jun 16, 2023

I have created a new project and its not crashing, but is crashing with a specific project with just that simple example as the entry point. So I guess there is a third party package that is conflicting, lets close this for now and I will look into it more and give a more detailed issue if I find the clash/cause.

The errors I get in sentry are not helpful either C++ Exception - UISystemKeyboardDockController

@kirillzyusko
Copy link
Owner

@owinter86 One thing that I can suggest is to try to call setupKVObserver/removeKVObserver in different lifecycle methods. Try to replace KeyboardMovementObserver with this content:

//
//  KeyboardMovementObserver.swift
//  KeyboardController
//
//  Created by Kiryl Ziusko on 2.08.22.
//  Copyright © 2022 Facebook. All rights reserved.
//

import Foundation

@objc(KeyboardMovementObserver)
public class KeyboardMovementObserver: NSObject {
  // class members
  var onEvent: (NSString, NSNumber, NSNumber) -> Void
  var onNotify: (String, Any) -> Void
  // progress tracker
  private var _keyboardView: UIView?
  private var keyboardView: UIView? {
    let windowsCount = UIApplication.shared.windows.count

    if _keyboardView == nil || windowsCount != _windowsCount {
      _keyboardView = findKeyboardView()
      _windowsCount = windowsCount
    }

    return _keyboardView
  }

  private var _windowsCount: Int = 0
  private var prevKeyboardPosition = 0.0
  private var displayLink: CADisplayLink?
  private var keyboardHeight: CGFloat = 0.0
  private var hasKVObserver = false

  @objc public init(
    handler: @escaping (NSString, NSNumber, NSNumber) -> Void,
    onNotify: @escaping (String, Any) -> Void
  ) {
    onEvent = handler
    self.onNotify = onNotify
  }

  @objc public func mount() {
    NotificationCenter.default.addObserver(
      self,
      selector: #selector(keyboardWillDisappear),
      name: UIResponder.keyboardWillHideNotification,
      object: nil
    )
    NotificationCenter.default.addObserver(
      self,
      selector: #selector(keyboardWillAppear),
      name: UIResponder.keyboardWillShowNotification,
      object: nil
    )
    NotificationCenter.default.addObserver(
      self,
      selector: #selector(keyboardDidAppear),
      name: UIResponder.keyboardDidShowNotification,
      object: nil
    )
    NotificationCenter.default.addObserver(
      self,
      selector: #selector(keyboardDidDisappear),
      name: UIResponder.keyboardDidHideNotification,
      object: nil
    )
  }

  private func setupKVObserver() {
    if hasKVObserver {
      return
    }

    if keyboardView != nil {
      hasKVObserver = true
      keyboardView?.addObserver(self, forKeyPath: "center", options: .new, context: nil)
    }
  }

  private func removeKVObserver() {
    if !hasKVObserver {
      return
    }

    hasKVObserver = false
    _keyboardView?.removeObserver(self, forKeyPath: "center", context: nil)
  }

  // swiftlint:disable:next block_based_kvo
  @objc override public func observeValue(
    forKeyPath keyPath: String?,
    of object: Any?,
    change: [NSKeyValueChangeKey: Any]?,
    context _: UnsafeMutableRawPointer?
  ) {
    // swiftlint:disable:next force_cast
    if keyPath == "center", object as! NSObject == _keyboardView! {
      // if we are currently animating keyboard or keyboard is not shown yet -> we need to ignore values from KVO
      if displayLink != nil || keyboardHeight == 0.0 {
        return
      }

      // swiftlint:disable:next force_cast
      let keyboardFrameY = (change?[.newKey] as! NSValue).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
      )

      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
      }

      onEvent("onKeyboardMoveInteractive", position as NSNumber, position / CGFloat(keyboardHeight) as NSNumber)
    }
  }

  @objc public func unmount() {
    // swiftlint:disable:next notification_center_detachment
    NotificationCenter.default.removeObserver(self)
  }

  @objc func keyboardWillAppear(_ notification: Notification) {
    if let keyboardFrame: NSValue = notification.userInfo?[UIResponder.keyboardFrameEndUserInfoKey] as? NSValue {
      let keyboardHeight = keyboardFrame.cgRectValue.size.height
      self.keyboardHeight = keyboardHeight

      var data = [AnyHashable: Any]()
      data["height"] = keyboardHeight

      onEvent("onKeyboardMoveStart", Float(keyboardHeight) as NSNumber, 1)
      onNotify("KeyboardController::keyboardWillShow", data)

      setupKeyboardWatcher()
    }
  }

  @objc func keyboardWillDisappear() {
    var data = [AnyHashable: Any]()
    data["height"] = 0

    onEvent("onKeyboardMoveStart", 0, 0)
    onNotify("KeyboardController::keyboardWillHide", data)

    setupKeyboardWatcher()
    removeKVObserver()
  }

  @objc func keyboardDidAppear(_ notification: Notification) {
    if let keyboardFrame: NSValue = notification.userInfo?[UIResponder.keyboardFrameEndUserInfoKey] as? NSValue {
      let keyboardHeight = keyboardFrame.cgRectValue.size.height
      self.keyboardHeight = keyboardHeight

      var data = [AnyHashable: Any]()
      data["height"] = keyboardHeight

      onEvent("onKeyboardMoveEnd", keyboardHeight as NSNumber, 1)
      onNotify("KeyboardController::keyboardDidShow", data)

      removeKeyboardWatcher()
      setupKVObserver()
    }
  }

  @objc func keyboardDidDisappear() {
    var data = [AnyHashable: Any]()
    data["height"] = 0

    keyboardHeight = 0.0

    onEvent("onKeyboardMoveEnd", 0 as NSNumber, 0)
    onNotify("KeyboardController::keyboardDidHide", data)

    removeKeyboardWatcher()
  }

  @objc func setupKeyboardWatcher() {
    // sometimes `will` events can be called multiple times.
    // To avoid double re-creation of listener we are adding this condition
    // (if active link is present, then no need to re-setup a listener)
    if displayLink != nil {
      return
    }

    displayLink = CADisplayLink(target: self, selector: #selector(updateKeyboardFrame))
    displayLink?.preferredFramesPerSecond = 120 // will fallback to 60 fps for devices without Pro Motion display
    displayLink?.add(to: .main, forMode: .common)
  }

  @objc func removeKeyboardWatcher() {
    displayLink?.invalidate()
    displayLink = nil
  }

  // https://stackoverflow.com/questions/32598490/show-uiview-with-buttons-over-keyboard-like-in-skype-viber-messengers-swift-i
  func findKeyboardView() -> UIView? {
    var result: UIView?

    let windows = UIApplication.shared.windows
    for window in windows {
      if window.description.hasPrefix("<UITextEffectsWindow") {
        for subview in window.subviews {
          if subview.description.hasPrefix("<UIInputSetContainerView") {
            for hostView in subview.subviews {
              if hostView.description.hasPrefix("<UIInputSetHostView") {
                result = hostView as? UIView
                break
              }
            }
            break
          }
        }
        break
      }
    }
    return result
  }

  @objc func updateKeyboardFrame() {
    if keyboardView == nil {
      return
    }

    let keyboardFrameY = keyboardView?.layer.presentation()?.frame.origin.y ?? 0
    let keyboardWindowH = keyboardView?.window?.bounds.size.height ?? 0
    let keyboardPosition = keyboardWindowH - keyboardFrameY

    if keyboardPosition == prevKeyboardPosition || keyboardFrameY == 0 {
      return
    }

    prevKeyboardPosition = keyboardPosition
    onEvent("onKeyboardMove", keyboardPosition as NSNumber, keyboardPosition / CGFloat(keyboardHeight) as NSNumber)
  }
}

Maybe it'll help 🙂

@owinter86
Copy link
Author

So it seems like if "@shopify/react-native-skia": "0.1.183", is installed it causes the crash, even though that snippet is all the app is loading.

@kirillzyusko kirillzyusko added 🍎 iOS iOS specific 🎯 crash Library triggers a crash of the app labels Jun 16, 2023
@kirillzyusko
Copy link
Owner

@owinter86 I've installed skia, installed pods, re-assembled project, and still didn't get a crash 😐

@owinter86
Copy link
Author

owinter86 commented Jun 16, 2023

I can reproduce on a new expo app, and new react native cli app with just these three dependencies on both sim and device, you need to make sure that if you are on the sim that the actual keyboard is visible though, so you may need to cmd + K to see the keyboard after the input has been focussed.

    "@shopify/react-native-skia": "0.1.183",
    "react-native-keyboard-controller": "1.5.6",
    "react-native-reanimated": "~2.14.4"

edit: upgrading reanimated and skia seems to fix the issue, though. Some others may run into this issue also as those are the "recommended" versions for the current Expo sdk 48 release.

edit2: seems like the current solution is to either remove skia, or to upgrade to reanimated V3

@kirillzyusko
Copy link
Owner

@owinter86 that's very weird. I have following in yarn.lock:

image image

And the issue is not reproducible 🤯 Would you mind to push your repo to GitHub? I'd love to check it out and see what is the problem there 👀

@owinter86
Copy link
Author

sure here is the repo, and the video below showing the crash only when the keyboard is visible.

https://github.com/owinter86/keyboard-crash-rn

Screen.Recording.2023-06-16.at.10.44.44.mov

@kirillzyusko
Copy link
Owner

@owinter86 I can confirm, the crash happens on iOS 16.5, but is not happening on iOS 16.2 🤷‍♂️

I'll investigate it more 👍

@kirillzyusko
Copy link
Owner

I found out a root problem. Need a little bit time to try various solutions to understand how would be better to fix it 👀

@kirillzyusko
Copy link
Owner

@owinter86 may I kindly ask you to test #178 and say whether it fixes the problem or not?

@owinter86
Copy link
Author

@kirillzyusko I can confirm this fixes the issue, nice work.

@kirillzyusko
Copy link
Owner

Thanks @owinter86

I'm going to do more tests on more devices and if everything is alright (which is most likely), then I'll merge this PR and prepare a new release quite soon (want to include some other fixes as well).

In a meantime you can patch-package your dependency or use library from the branch of this PR 🙂

kirillzyusko added a commit that referenced this issue Jun 21, 2023
## 📜 Description

Fixed a crash on iOS 16.5 when native alert is getting shown.

## 💡 Motivation and Context

The original exception occurred because of this error:

```bash
libc++abi: terminating due to uncaught exception of type folly::json::print_error: folly::toJson: JSON object value was an INF when serializing value at "progress"
```

`progress` was `inf` because in:

```swift
onEvent("onKeyboardMove", keyboardPosition as NSNumber, keyboardPosition / CGFloat(keyboardHeight) as NSNumber)
```

`keyboardHeight` variable was `0`. When we were trying to divide any
number by `0` in swfit we get `Inf` value, and this value can not be
serialised to JS equivalent.

The reason why this value becomes `0` is the fact that we have next
chain of calls:

```
keyboardWillHide
keyboardDidHide <- here we are setting keyboardHeight=0
keyboardWillHide <- here we setup CADisplayLink and after 16ms the crash will occur
keyboardDidHide
```

Based on output from above I had 3 options on how to fix the error:

### 1. Add `.isFinite` check

Good option, but it will prevent sending `onMove` event, though
according to values from `CADisplayLink` movement actually happens. So
we will kind of ignore actual information and it's not something that
satisfy me, because it may produce other bugs in the future.

### 2. Update `keyboardHeight` in `keyboardWillHide`

This variant also prevents a crash, but `progress` value in `onMove`
sometimes can be higher than `1` (in my tests sometimes it was 1.3+).
And it also seems like it gives incorrect calculation, because it's
strange to see progress > 1, when keyboard simply hides 🤷‍♂️

### 3. Don't set `keyboardHeight` to `0`

To come back to `1.4.x` behaviour, we don't need to reset
`keyboardHeight` to `0`. But I've added this code intentionally in
`1.5.0`, because in KV observer we are checking that keyboard should be
open and not animating:

```swift
if displayLink != nil || keyboardHeight == 0.0 {
  return
}
```

I've added this check, because I've attached KV observer before any
keyboard movement. The reason why I've added it in
`UIWindow.didBecomeVisibleNotification`/`UIWindow.didBecomeHiddenNotification`
was the fact, that these methods were called only one time (keyboard
events can be fired multiple times). But I've added `hasKVObserver`
variable in
#146
so it's safe to call `setupKVObserver`/`removeKVObserver` multiple
times.

So in this PR I've decided to stick to this approach - don't reset
`keyboardHeight` to `0` + setup KV listeners in different lifecycle
methods.

Should close
#175

## 📢 Changelog

### iOS
- moved `setupKVObserver`/`removeKVObserver` to
`keyboaardDidAppear`/`keyboardWilHide` lifecycle;
- don't set `keyboardHeight` to `0` (this value acts as
`persisteKeyboardHeight` on Android);

## 🤔 How Has This Been Tested?

Tested on:
- iPhone 6s (iOS 15.6);
- iPhone 14 Pro (iOS 16.2);
- iPhone 11 (iOS 16.5);

## 📸 Screenshots (if appropriate):

|Before|After|
|------|-----|
|<video
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/82d46951-3471-4b59-8dd6-3062922a76f0">|<video
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/b73d2aa2-72a9-4272-8b37-f6820d1c816a">|

## 📝 Checklist

- [x] CI successfully passed
@kirillzyusko
Copy link
Owner

Merged and published under 1.5.7 version 😊

In case if issue re-occurs - feel free to submit a new issue 👀

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 🍎 iOS iOS specific
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants