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 android platform border color #39893

Closed
wants to merge 3 commits into from

Conversation

axinvd
Copy link
Contributor

@axinvd axinvd commented Oct 6, 2023

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

Test Plan:

In RNTester example, go to APIs -> PlatformColor

Before After
drawing drawing

@facebook-github-bot
Copy link
Contributor

Hi @axinvd!

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 cla@meta.com. Thanks!

@analysis-bot
Copy link

analysis-bot commented Oct 6, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,317,608 +7
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,685,718 +4
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 6355703
Branch: main

@axinvd
Copy link
Contributor Author

axinvd commented Oct 6, 2023

Fixes:
#38950
#38614
#32942

@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 6, 2023
@javache
Copy link
Member

javache commented Oct 6, 2023

Thanks! Could you test this on the new architecture too?

@axinvd
Copy link
Contributor Author

axinvd commented Oct 6, 2023

I tested on rn-tester, where new arch enabled by default

@javache
Copy link
Member

javache commented Oct 6, 2023

The code you're changing only applies on the old architecture. The new architecture uses BaseViewManagerDelegate, which seems like it still has the same issue.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

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

Generated by 🚫 dangerJS against 97743ce

@axinvd
Copy link
Contributor Author

axinvd commented Oct 6, 2023

In BaseViewManagerDelegate setting native view properties like setBackgroundColor
BorderColor setting in ReactViewBackgroundDrawable

@facebook-github-bot
Copy link
Contributor

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

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

@javache merged this pull request in 265af22.

majkpal pushed a commit to majkpal/react-native that referenced this pull request Oct 17, 2023
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: facebook#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
(cherry picked from commit 265af22)
majkpal pushed a commit to majkpal/react-native that referenced this pull request Oct 17, 2023
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: facebook#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
(cherry picked from commit 265af22)
fortmarek pushed a commit that referenced this pull request Nov 8, 2023
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
hurali97 pushed a commit to hurali97/react-native that referenced this pull request Dec 19, 2023
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: facebook#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
huntie pushed a commit that referenced this pull request Dec 27, 2023
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
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 <axinvd@gmail.com>
Co-authored-by: Filipp Mikheev <kot331107@gmail.com>
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.

4 participants