Skip to content

Conversation

@kot331107
Copy link
Contributor

@kot331107 kot331107 commented Oct 9, 2023

Summary:

Fixes #40754

Hi all!
We noticed that our app started to crash after bumping to RN v0.71.13, anyways after a deeper investigation we also found that the crash occurs in the latest version as well.

Crash log:

E  FATAL EXCEPTION: main
Process: com.nfl.fantasy.core.android.debug, PID: 6034
java.lang.ClassCastException: android.app.ContextImpl cannot be cast to android.app.Activity
at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.getActivity(ReactRootView.java:926)
at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.checkForKeyboardEvents(ReactRootView.java:946)
at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.onGlobalLayout(ReactRootView.java:912)
at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061)

The code which causes ClassCastException is following here.
In this code explicit type conversion to Activity is not safe because it's not guaranteed by the compiler that context will be compatible with Activity type.
The appropriate issue has been filed.

Changelog:

[ANDROID] [FIXED] - Fixed crash occurring in certain native views when keyboard events are fired.

Test Plan:

Tested it manually with the reference application. Repro steps are as follows:

  • Build and run the app on Android
  • Tap the button "Open Modal"
  • You should see the red popup fragment to the bottom of the screen
  • Tap on the text input to open software keyboard
  • Expected: it should show the keyboard and no crash happens.

@facebook-github-bot
Copy link
Contributor

Hi @kot331107!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@analysis-bot
Copy link

analysis-bot commented Oct 10, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,222,901 +7
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,595,569 -4
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: f00594b
Branch: main

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 10, 2023
@github-actions
Copy link

github-actions bot commented Oct 10, 2023

Warnings
⚠️ One hour and a half have passed and the E2E jobs haven't finished yet.

Generated by 🚫 dangerJS against 693957c

@NickGerleman
Copy link
Contributor

What happens if you try to get the activity from the context of the result of "getRootView()", instead of the React root view?

@kot331107
Copy link
Contributor Author

kot331107 commented Oct 10, 2023

from the context of the result of "getRootView()"

It gives me same exception but with different type DecorContext

FATAL EXCEPTION: main
Process: com.helloworld, PID: 25841
java.lang.ClassCastException: com.android.internal.policy.DecorContext cannot be cast to android.app.Activity
                                                                                                    

Well, while debugging in ReactRootView I found that it hits twice in the getActivity() in my reference app where I follow repro steps. The first time it hits for the MainActivity and converts it successfully. The second time it calls getActivity() for Fragment which is used in our app.
And from the logic in ReactRootView.java my gut feeling says that this follow-up call of getActivity() is pretty redundant because in the first pass it calculates visibility of the keyboard, insets and raises needed events. So it should be safe to check if the result of getActivity() could be converted to Activity. And if it is not the case then ignore the following logic (which is done by returning null in this PR).
Besides this actual crash/exception, I think it's worth to add this type-safety check because it looks like a potential casting error in the current implementation.

@NickGerleman
Copy link
Contributor

In this scenario where we are called into twice, do we have multiple root views visible at once subscribing to these events?

If it's possible to get into this case, where we cannot resolve an activity to resolve the current window, we could ideally find a reliable and not to invasive way to get the current window softinputmode.

Searching around, I think we might be able to get this information off of the root DecorView in the hierarchy as WindowManager.LayoutParams, without needing to resolve an activity which created the view. https://stackoverflow.com/a/61768230

Could we maybe test that approach for resolving softInputMode?

@kot331107
Copy link
Contributor Author

@NickGerleman I haven't found a way to get it from the root DecorView but I'm able to handle my case by iterating getParent(). In this case I'm able to get MainActivity and then the subsequent logic works as well.

@kot331107
Copy link
Contributor Author

kot331107 commented Oct 11, 2023

oh wait, I think I found something more robust :D
UPD: my mistake. I tried to get it in the following way getRootViewGroup().getContext() but it returns me Application instead of Activity. So it seems the only way to go is to make it iterating getParent(). I'll push my changes shortly
UPD2: @NickGerleman well I updated PR. Now it finds the Activity (and then resolves window and its softInputMode) in the parent views for Fragments. I tested it on my ref app and it lgtm.

@facebook-github-bot
Copy link
Contributor

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

@NickGerleman
Copy link
Contributor

NickGerleman commented Oct 12, 2023

Just to check, the snippet in the above linked post didn't work for your scenario?

private WindowManager.LayoutParams tryGetWindowParams()
{
    View view = this;
    if (view.getLayoutParams() instanceof WindowManager.LayoutParams)
        return (WindowManager.LayoutParams) view.getLayoutParams();
    while (view.getParent() instanceof View)
    {
        view = (View) view.getParent();
        if (view.getLayoutParams() instanceof WindowManager.LayoutParams)
            return (WindowManager.LayoutParams) view.getLayoutParams();
    }
    return null;
}

@kot331107
Copy link
Contributor Author

kot331107 commented Oct 12, 2023

Just to check,

hey thanks for double checking! I got what you suggested. This works as well in my case. Thank you for your help @NickGerleman and please see the latest changes

UPD I merged with the latest from main and the test_android_template-... builds are failing now. I don't believe it is related to my changes 🤷‍♂️

int softInputMode = getActivity().getWindow().getAttributes().softInputMode;
int softInputMode;
Activity activity = getActivity();
if (activity != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We no longer need the activity, right?

@NickGerleman
Copy link
Contributor

Yeah, Android template issue seems unrelated.

Thanks for the work you’re doing here, I think after we remove the no longer needed code for getting an activity, we should merge this.

@kot331107
Copy link
Contributor Author

kot331107 commented Oct 12, 2023

@NickGerleman my pleasure doing something useful :D
I'll test it out in my app. And unluckily I don't have enough info about test cases they used when they have been working on getActivity stuff in this PR #36121 If you have it please let me know.

Ah I see, they used https://github.com/stripe/stripe-react-native for testing. I'm gonna try to setup and test with it as well.
@cortinico mb you have any quick test scenarios which I could quickly reproduce in a simple Android app to make sure it won't break anything on your end?

@kot331107
Copy link
Contributor Author

In my test app it works well after removing getActivity(). Trying to test out with stripes app as well

@NickGerleman
Copy link
Contributor

If local debugging shows you are able to get the root window params with this, and tests pass, I think we are probably fine. Ideally we'd have some better test coverage around keyboard events (we have seen issues there before, esp for older Android versions).

@kot331107
Copy link
Contributor Author

If local debugging shows you

yeah I checked with debugger, sure thing

@kot331107
Copy link
Contributor Author

it seems I should fix tests as well

@NickGerleman
Copy link
Contributor

TIL we have tests for this 😅. It looks like testCheckForKeyboardEvents It looks like we might just need to mock getWindowLayoutParams()

@kot331107
Copy link
Contributor Author

I wasn't able to setup tests in react-native quickly (gradle doesn't completely sync the project) so making a blind fix for tests :/

@cortinico
Copy link
Contributor

I wasn't able to setup tests in react-native quickly (gradle doesn't completely sync the project) so making a blind fix for tests :/

I'd say do the following:

  1. Open a terminal, launch ./gradlew build
  2. Make sure that build finishes
  3. Open in Android Studio
  4. Gradle Sync should work fine

@kot331107
Copy link
Contributor Author

@cortinico I think I tried doing it from terminal too. Anyways test_android was fixed and the remaining failing checks shouldn't be related to this pr cc @NickGerleman

@kot331107
Copy link
Contributor Author

Lemme try to merge it with the latest from main

@cortinico
Copy link
Contributor

/rebase

@github-actions github-actions bot force-pushed the android-fix-crash-in-rootview-on-softkeyboard branch from 2e04640 to 693957c Compare October 13, 2023 16:29
@kot331107
Copy link
Contributor Author

error Couldn't find any versions for "@react-native/normalize-colors" that matches "^0.73.0" the error is most likely related to RN, not to android native changes.

@kot331107
Copy link
Contributor Author

@cortinico btw, I resolved my local issues with building react-native by pinning the JDK version in gradle.properties. I have several JDK installed on my comp and that was the culprit.
org.gradle.java.home=/Library/Java/JavaVirtualMachines/amazon-corretto-17.jdk/Contents/Home

@facebook-github-bot
Copy link
Contributor

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

@github-actions
Copy link

This pull request was successfully merged by @kot331107 in 9497203.

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

@github-actions github-actions bot added the Merged This PR has been merged. label Oct 14, 2023
@NickGerleman
Copy link
Contributor

@kot331107 would you be able to give #40970 a try?

@kot331107 kot331107 deleted the android-fix-crash-in-rootview-on-softkeyboard branch October 14, 2023 19:56
hurali97 pushed a commit to hurali97/react-native that referenced this pull request Dec 19, 2023
…eyboard is shown (facebook#40755)

Summary:
Fixes facebook#40754

Hi all!
We noticed that our app started to crash after bumping to RN v0.71.13, anyways after a deeper investigation we also found that the crash occurs in the latest version as well.

Crash log:
```
E  FATAL EXCEPTION: main
Process: com.nfl.fantasy.core.android.debug, PID: 6034
java.lang.ClassCastException: android.app.ContextImpl cannot be cast to android.app.Activity
at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.getActivity(ReactRootView.java:926)
at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.checkForKeyboardEvents(ReactRootView.java:946)
at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.onGlobalLayout(ReactRootView.java:912)
at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061)
```
The code which causes ClassCastException is following [here](https://github.com/facebook/react-native/blob/ea88fbe229e1d276753ee8e118184274fc872138/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java#L864).
In this code explicit type conversion to Activity is not safe because it's not guaranteed by the compiler that context will be compatible with Activity type.
The appropriate issue [has been filed](facebook#40754).

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[ANDROID] [FIXED] - Fixed crash occurring in certain native views when keyboard events are fired.

Pull Request resolved: facebook#40755

Test Plan:
Tested it manually with the [reference application](https://github.com/kot331107/rnCrashReproducer).  Repro steps are as follows:

- Build and run the app on Android
- Tap the button "Open Modal"
- You should see the red popup fragment to the bottom of the screen
- Tap on the text input to open software keyboard
- Expected: it should show the keyboard and no crash happens.

Reviewed By: arushikesarwani94

Differential Revision: D50198424

Pulled By: NickGerleman

fbshipit-source-id: a5a6d86334856f4ffbe818150da5793380da4702
lunaleaps pushed a commit that referenced this pull request Jan 3, 2024
* Fix android platform border color (#39893)

Summary:
If you try to apply PlatformColor to borders on Android app will crash with the next error:

"Error while updating property 'borderColor' of a view managed by: RCTView"

## Changelog:

[ANDROID] [FIXED] - Fix android crash when apply PlatformColor to borders

Pull Request resolved: #39893

Test Plan:
In RNTester example, go to APIs -> PlatformColor
|    Before  | After |
| ----------- | ----------- |
|  <img src="https://github.com/facebook/react-native/assets/70860930/66ac2880-53da-4438-bd9a-332f8ea40645" alt="drawing" width="200"/>    | <img src="https://github.com/facebook/react-native/assets/70860930/151f58a1-d857-4b3d-9ec6-de74eb065127" alt="drawing" width="200"/>      |

Reviewed By: NickGerleman

Differential Revision: D50011758

Pulled By: javache

fbshipit-source-id: ea06c18c6aef4b6731e9b9b87422a1e0d13de208

* Android: fix ClassCastException in ReactRootView.java when software keyboard is shown (#40755)

Summary:
Fixes #40754

Hi all!
We noticed that our app started to crash after bumping to RN v0.71.13, anyways after a deeper investigation we also found that the crash occurs in the latest version as well.

Crash log:
```
E  FATAL EXCEPTION: main
Process: com.nfl.fantasy.core.android.debug, PID: 6034
java.lang.ClassCastException: android.app.ContextImpl cannot be cast to android.app.Activity
at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.getActivity(ReactRootView.java:926)
at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.checkForKeyboardEvents(ReactRootView.java:946)
at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.onGlobalLayout(ReactRootView.java:912)
at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061)
```
The code which causes ClassCastException is following [here](https://github.com/facebook/react-native/blob/ea88fbe229e1d276753ee8e118184274fc872138/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java#L864).
In this code explicit type conversion to Activity is not safe because it's not guaranteed by the compiler that context will be compatible with Activity type.
The appropriate issue [has been filed](#40754).

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[ANDROID] [FIXED] - Fixed crash occurring in certain native views when keyboard events are fired.

Pull Request resolved: #40755

Test Plan:
Tested it manually with the [reference application](https://github.com/kot331107/rnCrashReproducer).  Repro steps are as follows:

- Build and run the app on Android
- Tap the button "Open Modal"
- You should see the red popup fragment to the bottom of the screen
- Tap on the text input to open software keyboard
- Expected: it should show the keyboard and no crash happens.

Reviewed By: arushikesarwani94

Differential Revision: D50198424

Pulled By: NickGerleman

fbshipit-source-id: a5a6d86334856f4ffbe818150da5793380da4702

* chore: bump podfile.lock

---------

Co-authored-by: Ivan Alexandrov <[email protected]>
Co-authored-by: Filipp Mikheev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. 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.

Android: ClassCastException crashes the app in ReactRootView.java when software keyboard is shown

5 participants