Summary:
We've been encountering a crash in `runAnimationStep` with "Calculated frame index should never be lower than 0" https://github.com/facebook/react-native/issues/35766 with OnePlus/Oppo devices as well, but don't have one on hand to test.
This just works around the issue: if the time is before the start time of an animation, we shouldn't do anything anyways, so we just log a message instead of throwing while in production. We still throw in debug mode though for easier debugging.
### Hypothesis of the root cause
Based on stacktrace in https://github.com/facebook/react-native/issues/35766 (which is the same one we see)
Normally, this should happen
1. Choreographer.java constructs a FrameDisplayEventReceiver
2. FrameDisplayEventReceiver.onVSync gets called, which sets the `mTimestampNanos`
3. FrameDisplayEventReceiver.run gets called, which then eventually calls our `doFrame` callback with `mTimestampNanos`. This then causes `FrameBasedAnimationDriver.runAnimationStep` to be called with the same timestamp
I suspect what's happening on OnePlus devices is that the `onVSync` call either doesn't happen or happens rarely enough that the `mTimestampNanos` when `run` is called is sometime in the past
### Fix
1. Add logging so we get the parameters to debug more if we end up getting this error
2. In production, just ignore past times instead of throwing an Error
## Changelog:
Pick one each for the category and type tags:
[ANDROID] [FIXED] - Prevent crash on OnePlus/Oppo devices in runAnimationStep
Pull Request resolved: https://github.com/facebook/react-native/pull/37487
Test Plan: Ran our app using patched version and verified no issues showed up when using it
Reviewed By: cipolleschi
Differential Revision: D46102968
Pulled By: cortinico
fbshipit-source-id: bcb36a0c2aed0afdb8e7e68b141a3db4eb02695a
Summary:
This is an attempt to fix the broken `test_buck` as the `/textinput` target now needs to access the Kotlin stdlib dependencies
## Changelog
[Internal] - Fix test_buck by providing exported deps for textinput target
Pull Request resolved: https://github.com/facebook/react-native/pull/34436
Test Plan: Will wait for a CircleCI result
Reviewed By: cipolleschi
Differential Revision: D38782473
Pulled By: cortinico
fbshipit-source-id: 72265c34092372189d75df732b64a1e370453472
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36575
This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior.
D23670779 addedd a previous mechanism to add spans for measurement caching, like we needed to do as part of this change. It is called in more specific cases (e.g. when there is a text hint but no text), but it edits the live EditText spannable instead of the cache copy, and does not handle nested text at all.
We are already adding spans back to the input after this, behind everything else, and can replace it with the code we have been adding.
Changelog:
[Android][Fixed] - Mimimize EditText Spans 9/9: Remove `addSpansForMeasurement()`
Reviewed By: javache
Differential Revision: D44298159
fbshipit-source-id: 1af44a39de7550b7e66e45db9ebc3523ae9ff002
# Conflicts:
# ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36577
This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior.
This change allows us to strip CustomStyleSpan. We already set all but `fontVariant` on the underlying EditText, so we just need to route that through as well.
Note that because this span is non-parcelable, it is seemingly not subject to the buggy behavior on Samsung devices of infinitely cloning the spans, but non-parcelable spans have different issues on the devices (they disappear), so moving `fontVariant` to the top-level makes sense here.
Changelog:
[Android][Fixed] - Minimize EditText Spans 8/N: CustomStyleSpan
Reviewed By: javache
Differential Revision: D44297384
fbshipit-source-id: ed4c000e961dd456a2a8f4397e27c23a87defb6e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36576
This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior.
This change addresses some minor CR feedback and removes the temporary list of spans in favor of applying them directly.
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D44295190
fbshipit-source-id: bd784e2c514301d45d0bacd8ee6de5c512fc565c
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36548
This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior.
This change lets us set `letterSpacing` on the EditText instead of using our custom span.
Changelog:
[Android][Fixed] - Minimize EditText Spans 6/N: letterSpacing
Reviewed By: rshest
Differential Revision: D44240777
fbshipit-source-id: 9bd10c3261257037d8cacf37971011aaa94d1a77
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36544
This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior.
This change makes us apply strikethrough and underline as paint flags to the underlying EditText, instead of just the spans. We then opt ReactUnderlineSpan and ReactStrikethroughSpan into being strippable.
This does actually create visual behavior changes, where child text will inherit any underline or strikethrough of the root EditText (including if the child specifies `textDecorationLine: "none"`. The new behavior is consistent with both iOS and web though, so it seems like more of a bugfix than a regression.
Changelog:
[Android][Fixed] - Minimize Spans 5/N: Strikethrough and Underline
Reviewed By: rshest
Differential Revision: D44240778
fbshipit-source-id: d564dfc0121057a5e3b09bb71b8f5662e28be17e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36545
This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior.
This adds ReactForegroundColorSpan to the list of spans eligible to be stripped.
Changelog:
[Android][Fixed] - Minimize Spans 4/N: ReactForegroundColorSpan
Reviewed By: javache
Differential Revision: D44240780
fbshipit-source-id: d86939cc2d7ed9116a4167026c7d48928fc51757
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36547
This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior.
This adds `ReactBackgroundColorSpan` to the list of spans eligible to be stripped.
Changelog:
[Android][Fixed] - Minimize Spans 3/N: ReactBackgroundColorSpan
Reviewed By: javache
Differential Revision: D44240782
fbshipit-source-id: 2ded1a1687a41cf6d5f83e89ffadd2d932089969
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36546
This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior.
This change generalizes `stripAttributeEquivalentSpans()` to allow plugging in different spans.
Changelog:
[Internal]
Reviewed By: rshest
Differential Revision: D44240781
fbshipit-source-id: 89005266020f216368e9ad9ce382699bd8db85a8
# Conflicts:
# ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36543
This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior.
We cache the backing EditText span on text change to later measure. To measure outside of a TextInput we need to restore any spans we removed. Spans may overlap, so base attributes should be behind everything else.
The logic here for dealing with precedence is incorrect, and we should instead accomplish this by twiddling with the `SPAN_PRIORITY` bits.
Changelog:
[Android][Fixed] - Minimize Spans 1/N: Fix precedence
Reviewed By: javache
Differential Revision: D44240779
fbshipit-source-id: f731b353587888faad946b8cf1e868095cdeced3
Summary:
D42721684 (https://github.com/facebook/react-native/commit/be69c8b5a77ae60cced1b2af64e48b90d9955be5) left a pretty bad bug when using Fabric for Android. I missed that in Fabric specifically, on edit we will cache the Spannable backing the EditText for use in future measurement.
Because we've stripped the sizing spans, Spannable measurement has incorrect font size, and the TextInput size will change (collapsing) after the first edit. This effectively breaks any uncontrolled TextInput which does not have explicit dimensions set.
Changelog:
[Android][Fixed] - Fix measurement of uncontrolled TextInput after edit
Reviewed By: sammy-SC
Differential Revision: D43158407
fbshipit-source-id: 51602eab06c9a50e2b60ef0ed87bdb4df025e51e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/35967
In https://github.com/facebook/react-native/issues/35936 we observed that the presence of AbsoluteSizeSpan may lead to hangs when using the Grammarly keyboard on Samsung.
This mitigation makes it so that we do not emit this span in any case where it is sufficient to rely on already set EditText textSize. In simple cases, tested on two devices, it causes typing into the TextInput to no longer hang.
This does not fully resolve the issue for TextInputs which meaningfully use layout-effecting spans (or at least font size), such as non-uniform text size within the input. We instead just try to reduce to minimum AbsoluteSizeSpan possible.
Testing the first commit was able to resolve hangs in some simpler inputs tested, by me and cortinico.
Changelog:
[Android][Fixed] - Mitigation for Samsung TextInput Hangs
Reviewed By: cortinico
Differential Revision: D42721684
fbshipit-source-id: e0388dfb4617f0217bc1d0b71752c733e10261dd
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/34927
The changes made in D36902220 (https://github.com/facebook/react-native/commit/a04195167bbd8f27c6141c0239a61a345cac5a88) and D36958882 (https://github.com/facebook/react-native/commit/d8c25ca1b62df2b93f70bbb1f7b379643ab9ccd4) attempted to reduce flickering and consistency issues when using Animated.
In the old renderer, we explicitly reset all animated props, and wait for the subsequent React commit to set the props to the right state, but if `initialProps` are used, the React reconciliation may not be able to identify the prop-update is required and will leave out the value. This behaviour is different in the new renderer, where we do not explicitly `restoreDefaultValues` on detaching the animated node, and instead rely on the latest state being correct(?).
Changelog:
[General][Fixed] Stop styles from being reset when detaching Animated.Values in old renderer
Fixes#34665
Reviewed By: rshest
Differential Revision: D40194072
fbshipit-source-id: 1b3fb1d1f4a39036a501a8a21e57002035dd5659
# Conflicts:
# Libraries/Animated/createAnimatedComponent.js
Summary:
CMake on Windows uses "Visual Studio **" generators are [multi-configuration](https://cgold.readthedocs.io/en/latest/glossary/multi-config.html),
which requires the hermes cli and hermes runtime build flavors to be compatible. Using the "MakeFile" generator will avoid the requirement and keeps the gradle script simpler. Please note that the "Makefile" generator is the default when only "Visual Studio Community edition" with default packages is available.
Use NMake generator for Hermes build on Windows
## Changelog
CMake on Windows uses "Visual Studio **" generators are [multi-configuration](https://cgold.readthedocs.io/en/latest/glossary/multi-config.html),
which requires the hermes cli and hermes runtime build flavors to be compatible. Using the "MakeFile" generator will avoid the requirement and keeps the gradle script simpler. Please note that the "Makefile" generator is the default when only "Visual Studio Community edition" with default packages is available.
[Android] [Fixed] - Use NMake generator for Hermes build on Windows
Pull Request resolved: https://github.com/facebook/react-native/pull/34790
Test Plan: Verified that RN build on Windows with multiple SKUs of Visual studio.d
Reviewed By: cortinico
Differential Revision: D39889332
Pulled By: cortinico
fbshipit-source-id: f2d39d981e7c58c8cb93a4322155c844cb6bded3
Summary:
Ensuring the file paths uses forward slashes as separator to avoid the characters from being interpreted as ASCII escape characters.
## Changelog
Fixing build failure building RN codegen CLI on Windows. Ensuring the file paths uses forward slashes as separator to avoid the characters from being interpreted as ASCII escape characters.
[Android] [Fixed] - Fixing failure building RN codegen CLI on Windows
Pull Request resolved: https://github.com/facebook/react-native/pull/34791
Test Plan: Ensured RN main branch builds on Windows with new architecture turned on.
Reviewed By: cipolleschi
Differential Revision: D39889468
Pulled By: cipolleschi
fbshipit-source-id: 7d79eac9f433908cc86dd7ca2eec841739ef6365
# Conflicts:
# ReactAndroid/cmake-utils/ReactNative-application.cmake
Summary:
when specifying dev server without port, e.g. http://www.example.com/, there are some issues.
1. redbox error
<img src="https://user-images.githubusercontent.com/46429/190540390-8ee420f2-7642-427b-9f2e-e0c6d31015f8.png" width="30%">
2. showing -1 in loading view
<img src="https://user-images.githubusercontent.com/46429/190540727-158f35ad-359f-443a-a4b0-768dd2f7e400.png" width="50%">
the root cause is coming from [`java.net.URL.getPort()` will return -1 when the url doesn't have a port](https://developer.android.com/reference/java/net/URL#getPort()). this pr replaces the parser to [`okhttp3.HttpUrl`](https://square.github.io/okhttp/4.x/okhttp/okhttp3/-http-url/#port) that it will have default port 80 for http or port 443 for https. the two call paths should only serve http/https address, not file:// address. it should be safe to change from java.net.URL to okhttp3.HttpUrl.
not fully related, in the case above, android will connect to `ws://www.example.com/:8097` for react-devtools
we should strip the trailing slash in *setUpReactDevTools.js*
## Changelog
[Android] [Fixed] - Fix port as -1 if dev server without specifying port on Android
Pull Request resolved: https://github.com/facebook/react-native/pull/34705
Test Plan:
test on rn-tester with the following steps
1. `yarn start`
2. open another terminal and run `ngrok http 8081` and it will return a tunnel url, e.g. `71a1-114-36-194-97.jp.ngrok.io`
3. open dev setting in app and change the dev server to `71a1-114-36-194-97.jp.ngrok.io`
5. reload the app
Reviewed By: cipolleschi
Differential Revision: D39573988
Pulled By: cortinico
fbshipit-source-id: 397df90ab30533207bd87a3f069132d97c22c7fd
Summary:
PlatformColor should work on all *color style attributes on all platform.
Partially fixes https://github.com/facebook/react-native/issues/32942
## Changelog
[Android] [Fixed] - Support PlatformColor in borderColor
Pull Request resolved: https://github.com/facebook/react-native/pull/33544
Test Plan: Open rn tester (USE_FABRIC=false) platform color api examples. Without the changes to ViewProps.java, it will error out.
Reviewed By: lunaleaps
Differential Revision: D39413519
Pulled By: NickGerleman
fbshipit-source-id: 58962ba2956aa3df45144108eec194aedf23886b
Summary:
When the webview is updated by play store, all apps that has loaded webview gets killed. For background see https://issuetracker.google.com/issues/228611949?pli=1
We have a long-running app and don't want this to happen and while we are not using webview anywhere, we are using websockets that loads it for reading cookies, however we are not using cookies to authenticate the websocket and thus want to disable the webview to avoid loading it unnecessarily and then getting killed unnecessarily.
The webview has support for beeing disabled https://developer.android.com/reference/android/webkit/WebView#disableWebView() however as-is this crashes React Native. It seems to me like this case is very similar to not having web view installed and should be handled in the same way.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[Android] [Fixed] - Avoid crash in ForwardingCookieHandler if webview is disabled
Pull Request resolved: https://github.com/facebook/react-native/pull/34483
Test Plan: Add `WebView.disableWebView();` as the first line in `MainActivity.onCreate`.
Reviewed By: christophpurrer
Differential Revision: D38981827
Pulled By: cipolleschi
fbshipit-source-id: 335a8420568ad0c80b834ae8a3b164e55ebe26f3
Summary:
`JNIEnv`'s `FindClass(..)` function takes the classes in the standard
`foo/bar/Baz` class specification (unless they're special, like arrays).
Specifying them with `Lfoo/bar/Baz;` results in a
`ClassNotFoundException` being raised -- which is especially unhelpful
when intending to re-throw an exception.
The docs for `JNIEnv#FindClass(..)` can be found [here][jnienv].
[jnienv]:
https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#:~:text=The%20name%20argument,java/lang/String%22
## Changelog
[Android] [Fixed] - Correctly resolve classes with FindClass(..)
Pull Request resolved: https://github.com/facebook/react-native/pull/34533
Test Plan:
No exact test plan. However, if an exception is thrown during the
rendering process, a fatal `ClassNotFoundException` is raised, rather
than having the error be passed up to the `ReactNativeManager`.
Fixes: facebook/yoga#1120
Reviewed By: amir-shalem
Differential Revision: D39133326
Pulled By: jacdebug
fbshipit-source-id: 86283b7d21aed49ed0e9027b2aef85f0108cdf9a
Summary:
Fixes https://github.com/facebook/react-native/issues/34229
Basically, the react android conflicts with the kotlin version that's defined in the top level build.gradle. To resolve it, the approach was to get the `kotlinVersion` defined in top level build.gradle and use it. If it's not defined, we use the default(1.6.10 as of now).
The reason behind not using the DSL is that it doesn't allow us to use the variables due to it's constrained syntax.
See [here](https://docs.gradle.org/current/userguide/plugins.html#sec:constrained_syntax)
So the idea was to use the build script to resolve the kotlin plugin and it works 👍 .
Kindly asking for review cortinico :)
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[Android] [Changed] - refactored usage of kotlin plugin
Pull Request resolved: https://github.com/facebook/react-native/pull/34255
Test Plan: Ran the node ./scripts/run-ci-e2e-tests.js --js --android --ios to verify it doesn't introduce any unexpected issues.
Reviewed By: dmitryrykun
Differential Revision: D38468567
Pulled By: cortinico
fbshipit-source-id: f9ab635fcf033f1d337ed90793ba1667957b8e01
Summary:
This Pull Request aims at removing the making of reactnativeutilsjni as it is built from the same sources as reactnativejni. It also replaces references to reactnativeutilsjni with reactnativejni.
This should get rid of `reactnativeutilsjni.so` while reusing `reactnativejni.so` in it's place. This should give us some size improvements in the finally built apk.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[Android] [Changed] - Replaced reactnativeutilsjni with reactnativejni in the build process to reduce size
Pull Request resolved: https://github.com/facebook/react-native/pull/34339
Test Plan:
1. Ran the CMakelist.txt file using CMake and I could see that reactnativeutilsjni.dir is no longer generated with my changes.
2. Built the aar from this branch in Android Studio and build happened successfully.
I am not sure if we could run any more tests. Please let me know in case anymore testing is required and I can do accordingly
Reviewed By: cortinico
Differential Revision: D38400481
Pulled By: genkikondo
fbshipit-source-id: 592736e56441328389ae89135667c336ff8018e6
Summary:
While I was working on rewriting `react-native-slider` to Fabric I found a weird bug that prevented the slider to be set as disabled (to be exact: call the method `slider.setEnabled(false)`. As it turned out the `accessibilityState` (with value: `accessibilityState={{disabled: true}}` prop occurred after the `enabled={false}` prop that I was passing to the slider, which lead to both of this props overwrite each other.
Handling of `accessibilityState` props inside view leads to always overwriting the enabled prop to true (even if we explicitly set it to `{disabled: false}`.
Workaround for this was to reorder the props, so that the `accesibilityState` occur before `disabled`, but I think it's better to not set `view.setEnabled(true)` if we are passing a disabled property.
## Changelog
[Android] [Fixed] - Fix accessibilityState overwriting view's disabled state on Android
Pull Request resolved: https://github.com/facebook/react-native/pull/34287
Test Plan:
Change order of props inside native component implementation (that `disabled` occurs before `accesibilityState`). For example: `Libraries/Components/Slider/Slider.js`
<details>
<summary>Video showing the bug in RNTester (using Switch component)</summary>
https://user-images.githubusercontent.com/52801365/181287547-964f50e2-55dc-450f-b413-0d1c14d4bb83.mp4
</details>
Reviewed By: NickGerleman
Differential Revision: D38209232
Pulled By: dmitryrykun
fbshipit-source-id: 93d423716f89b45251be9d5aefcf01f7bd776f2c
Summary:
This cleans up a bit our dependency list inside `ReactAndroid`. I've re-sorted the lists, and exported everything I could in the `gradle.properties`.
I wish we could move to the Gradle Version Catalog for Android, but that's not possible at the moment (it will make impossible for users to build from source).
## Changelog
[Internal] - Cleanup some of the dependencies inside ReactAndroid
Pull Request resolved: https://github.com/facebook/react-native/pull/34191
Test Plan: Will rely on a Green CircleCI
Reviewed By: huntie
Differential Revision: D37828614
Pulled By: cortinico
fbshipit-source-id: f433d08d691db4145e0c72ca4dab2f0350e4101f
Riccardo Cipolleschi
Distiller
Distiller
Distiller
Distiller
Harry Yu
Distiller
Nicola Corti
Pieter De Baets
Nick Gerleman
Joshua Gross
Distiller
Distiller
Distiller
Lorenzo Sciandra
Distiller
Distiller
Distiller
Distiller
Anandraj Govindan
Distiller
Kudo Chien
Distiller
Danilo Bürger
Rasmus Eneman
Evan Charlton
Distiller
Distiller
Distiller
Muhammad Hur Ali
Distiller
Sparsha Saha
Oskar Kwaśniewski
Mike Hardy
Distiller
