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 action sheet callback invoked more than once on iPad #33099

Closed
wants to merge 2 commits into from

Conversation

janicduplessis
Copy link
Contributor

Summary

iOS will sometimes invoke the UIAlertAction handler for the cancel button more than once on iPad. This can be reproduced relatively easily by having a button that opens an action sheet and spam tapping outside the action sheet while it is opening. Since native module callbacks can only be invoked once this causes the app to crash here https://github.com/facebook/react-native/blob/main/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm#L206.

Changelog

[iOS] [Fixed] - Fix action sheet callback invoked more than once on iPad

Test Plan

Tested on iPad simulator to reproduce the crash and verified that this fixes it.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Feb 13, 2022
@react-native-bot react-native-bot added Bug Platform: iOS iOS applications. labels Feb 13, 2022
@analysis-bot
Copy link

analysis-bot commented Feb 13, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: f89a0b7
Branch: main

@analysis-bot
Copy link

analysis-bot commented Feb 13, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,189,108 +0
android hermes armeabi-v7a 7,789,539 +0
android hermes x86 8,559,204 +0
android hermes x86_64 8,511,588 +0
android jsc arm64-v8a 9,857,605 +0
android jsc armeabi-v7a 8,842,911 +0
android jsc x86 9,824,292 +0
android jsc x86_64 10,420,764 +0

Base commit: f89a0b7
Branch: main

@facebook-github-bot
Copy link
Contributor

@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

// The handler for a button might get called more than once when tapping outside
// the action sheet on iPad. RCTResponseSenderBlock can only be called once so
// keep track of callback invocation here.
__block bool callbackInvoked = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is never set to true? the condition in L146 will always pass. am i missing something obvious?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're right, my bad I removed it to test before / after and forgot to put it back -_-

@facebook-github-bot
Copy link
Contributor

@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @janicduplessis in 8935d6e.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Feb 16, 2022
@janicduplessis janicduplessis deleted the patch-12 branch February 16, 2022 15:18
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 15, 2023
)

Summary:
iOS will sometimes invoke the UIAlertAction handler for the cancel button more than once on iPad. This can be reproduced relatively easily by having a button that opens an action sheet and spam tapping outside the action sheet while it is opening. Since native module callbacks can only be invoked once this causes the app to crash here https://github.com/facebook/react-native/blob/main/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm#L206.

## Changelog

[iOS] [Fixed] - Fix action sheet callback invoked more than once on iPad

Pull Request resolved: facebook#33099

Test Plan: Tested on iPad simulator to reproduce the crash and verified that this fixes it.

Reviewed By: philIip

Differential Revision: D34215327

Pulled By: ShikaSD

fbshipit-source-id: 6f406e4df737a57e6dd702dd54260aa72eab31d6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. Merged This PR has been merged. Platform: iOS iOS applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants