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.

Sign up for GitHub

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

Jump to bottom

iOS 15 only : Alert leaves behind invisible window that steals all user interaction #32304

Closed
paddlefish opened this issue Sep 30, 2021 · 7 comments
Closed

iOS 15 only : Alert leaves behind invisible window that steals all user interaction #32304

paddlefish opened this issue Sep 30, 2021 · 7 comments
Labels
Resolution: Fixed A PR that fixes this issue has been merged. Resolution: Locked This issue was locked by the bot.

Comments

@paddlefish
Copy link
Contributor

Please provide all the information requested. Issues that do not follow this format are likely to stall.

Description

Something changed about how UIWindow and the root view controller interact. the _alertWindow created in RCTAlertController is no longer removed from the screen when nil'd in - (void) hide. I suspect that somewhere a retain cycle was introduced by the iOS 15 runtime.

React Native version:

System:
    OS: macOS 11.6
    CPU: (8) x64 Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz
    Memory: 46.57 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.17.5 - ~/.nvm/versions/node/v14.17.5/bin/node
    Yarn: 1.22.11 - ~/.nvm/versions/node/v14.17.5/bin/yarn
    npm: 7.23.0 - ~/.nvm/versions/node/v14.17.5/bin/npm
    Watchman: 2021.06.07.00 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.11.2 - /Users/arahn/.rvm/gems/ruby-2.6.3/bin/pod
  SDKs:
    iOS SDK:
      Platforms: iOS 15.0, DriverKit 20.4, macOS 11.3, tvOS 15.0, watchOS 8.0
    Android SDK: Not Found
  IDEs:
    Android Studio: 4.2 AI-202.7660.26.42.7351085
    Xcode: 13.0/13A233 - /usr/bin/xcodebuild
  Languages:
    Java: 1.8.0_65 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 17.0.2 => 17.0.2 
    react-native: 0.65.1 => 0.65.1 
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps To Reproduce

  1. Call Alert.alert, e.g.
    InteractionManager.runAfterInteractions(() => {
      Alert.alert('Delete this post?', 'Are you sure you want to permanently delete this?', [
        {
          text: 'Cancel',
          style: 'cancel',
        },
        {
          onPress: () => someFunction(),
          text: 'Delete',
          style: 'destructive',
        },
      ])
    })
  1. Run code and dismiss alert.
  2. Use XCode to pause app execution an look at UIView hierarchy.

Expected Results

You can interact with the app still

There are no extra windows still on the screen after alert is dismissed

Actual Results

all user interaction is blocked. There is a Window and a UIViewController still on the screen.
@paddlefish paddlefish mentioned this issue Sep 30, 2021
Closed
@asafkorem
Copy link
Contributor

asafkorem commented Dec 28, 2021 •
edited
Loading

I am able to reproduce this also. With both iOS 15 and iOS 14.5.

@asafkorem
Copy link
Contributor

asafkorem commented Dec 28, 2021 •
edited
Loading

That's a serious issue here, it also breaks automation tests when using Detox (and a real bug generally). 🔴 🔴 🔴
Note that the overlay seems to disappear after the first touch gesture attempt (only the first gesture is blocked).

@MoOx
Copy link
Contributor

MoOx commented Dec 29, 2021

I confirm this is a serious issue very annoying for user. I am facing it using RN 0.64.2.

asafkorem added a commit to asafkorem/react-native that referenced this issue Jan 5, 2022
@asafkorem
fix(iOS): remove alert's window when call to hide.
8019bb9 
Resolves this issue: facebook#32304.

Without this change, calling to hide an alert, leaves a `UIWindow`
that blocks user interactions with the screen.

The correct way to remove a `UIWindow` in iOS is to set its
hidden property to `YES`.

Also, it is required to remove all references to the window (the
associated `windowScene` for example) and ARC will automatically
free this `UIWindow`.
asafkorem added a commit to asafkorem/react-native that referenced this issue Jan 5, 2022
@asafkorem @paddlefish
fix(iOS): remove alert's window when call to hide.
2084c55 
Resolves this issue: facebook#32304.

Without this change, calling to hide an alert, leaves a `UIWindow`
that blocks user interactions with the screen.

The correct way to remove a `UIWindow` in iOS is to set its
hidden property to `YES`.

Also, it is required to remove all references to the window (the
associated `windowScene` for example) and ARC will automatically
free this `UIWindow`.

Co-authored-by: paddlefish <paddlefish@users.noreply.github.com>
@asafkorem asafkorem mentioned this issue Jan 5, 2022
Closed
asafkorem added a commit to asafkorem/react-native that referenced this issue Jan 5, 2022
@asafkorem @paddlefish
fix(iOS): remove alert's window when call to hide.
85fa844 
Resolves this issue: facebook#32304.

Without this change, calling to hide an alert, leaves a `UIWindow`
that blocks user interactions with the screen.

The correct way to remove a `UIWindow` in iOS is to set its
hidden property to `YES`.

Also, it is required to remove all references to the window (the
associated `windowScene` for example) and ARC will automatically
free this `UIWindow`.

Co-authored-by: paddlefish <paddlefish@users.noreply.github.com>
asafkorem added a commit to asafkorem/react-native that referenced this issue Jan 5, 2022
@asafkorem @paddlefish
fix(iOS): remove alert's window when call to hide.
3767095 
Resolves this issue: facebook#32304.

Without this change, calling to hide an alert, leaves a `UIWindow`
that blocks user interactions with the screen.

The correct way to remove a `UIWindow` in iOS is to set its
hidden property to `YES`.

Also, it is required to remove all references to the window (the
associated `windowScene` for example) and ARC will automatically
free this `UIWindow`.

Co-authored-by: paddlefish <paddlefish@users.noreply.github.com>
facebook-github-bot pushed a commit that referenced this issue Jan 19, 2022
@asafkorem @facebook-github-bot
fix(iOS): remove alert's window when call to hide. (#32833)
a46a99e 
Summary:
Resolves this issue: #32304.
**NOTE:** This PR is based on a prior PR for this fix: #32305, I've co-authorized its creator for this change (paddlefish).

Without this change, calling to hide an alert, leaves a `UIWindow` that blocks user interactions with the screen.

The correct way to remove a `UIWindow` in iOS is to set its hidden property to `YES`. Also, it is required to remove all references to the window (the associated `windowScene` for example) and ARC will automatically free this `UIWindow`.

The line after this change, set the `_alertWindow` reference to `nil`, but the window is already associated with a scene (see the screenshots from [this PR](#32305 (comment))). So we also need to remove the `windowScene` from that window, as recommended by Apple: https://developer.apple.com/documentation/uikit/uiwindowscene/3198091-windows.
>To remove the window from the current scene, or move it to a different scene, change the value of the window's windowScene property.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[iOS] [Fixed] - remove alert's window when call to `hide`.

Pull Request resolved: #32833

Test Plan: See #32305

Reviewed By: hramos

Differential Revision: D33460430

Pulled By: lunaleaps

fbshipit-source-id: b13c2c7ee6404f1e1c787265bc4af8a31005bcf1
lunaleaps pushed a commit that referenced this issue Jan 20, 2022
@asafkorem
fix(iOS): remove alert's window when call to hide. (#32833)
218b4c1 
Summary:
Resolves this issue: #32304.
**NOTE:** This PR is based on a prior PR for this fix: #32305, I've co-authorized its creator for this change (paddlefish).

Without this change, calling to hide an alert, leaves a `UIWindow` that blocks user interactions with the screen.

The correct way to remove a `UIWindow` in iOS is to set its hidden property to `YES`. Also, it is required to remove all references to the window (the associated `windowScene` for example) and ARC will automatically free this `UIWindow`.

The line after this change, set the `_alertWindow` reference to `nil`, but the window is already associated with a scene (see the screenshots from [this PR](#32305 (comment))). So we also need to remove the `windowScene` from that window, as recommended by Apple: https://developer.apple.com/documentation/uikit/uiwindowscene/3198091-windows.
>To remove the window from the current scene, or move it to a different scene, change the value of the window's windowScene property.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[iOS] [Fixed] - remove alert's window when call to `hide`.

Pull Request resolved: #32833

Test Plan: See #32305

Reviewed By: hramos

Differential Revision: D33460430

Pulled By: lunaleaps

fbshipit-source-id: b13c2c7ee6404f1e1c787265bc4af8a31005bcf1
@asafkorem
Copy link
Contributor

asafkorem commented Jan 25, 2022 •
edited
Loading

This issue was resolved in RN v.67.1.
Thanks @paddlefish

@MoOx
Copy link
Contributor

MoOx commented Jan 25, 2022

Yep, same finding, RN 0.67 fixed this

@paddlefish
Copy link
Contributor Author

Thanks to all involved. That was an interesting journey.

@cortinico
Copy link
Contributor

Yep, same finding, RN 0.67 fixed this

Closing as this was solved in #32833

@cortinico cortinico added Resolution: Fixed A PR that fixes this issue has been merged. and removed Needs: Triage 🔍 labels Jan 25, 2022
@facebook facebook locked as resolved and limited conversation to collaborators Jan 25, 2023
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jan 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Resolution: Fixed A PR that fixes this issue has been merged. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@MoOx @paddlefish @cortinico @react-native-bot @asafkorem