Summary:
Blocking people didn't work in Dating Settings without the Bridge.
Changelog: [Internal]
Reviewed By: ejanzer
Differential Revision: D23904867
fbshipit-source-id: 4a68b9d99fcc812f6616783a06dc047a3bc64491
Summary:
TurboModule Java files are still using the old lib name: `turbomodulejsijni`, so let's keep it that way for now.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D23946945
fbshipit-source-id: ff095ff51dca532c82e67e1c75e9a4e9be392d61
Summary:
in Android 11, there's an issue where Text content is being clipped. The root cause appears to be a breaking change in how Android 11 is measuring text. rounding up the layoutWidth calculation mitigates the issue.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D23944772
fbshipit-source-id: 1639259da1c2c507c6bfc80fed377577316febac
Summary:
Changelog: [Internal]
Padding needs to be rounded down (floor function), not rounded to the closest natural number.
Otherwise the content of the view might not fit.
Reviewed By: JoshuaGross
Differential Revision: D23905145
fbshipit-source-id: e84d70155b207144b98646dd0c4fea7a8c4bd876
Summary:
Changelog: [internal]
When paper measures text, it sets `useLineSpacingFromFallbacks` flag on the Layout object. In Fabric this was missing and can cause incorrect layout.
Reviewed By: JoshuaGross
Differential Revision: D23845441
fbshipit-source-id: 538f440cdbbf8df2cba0458837b80db103888113
Summary: Adding two new ReactMarkers for start and end of bridgeless initialization. Creating new ones instead of reusing existing ones to make it easier to differentiate data from the bridge vs. bridgeless, and also because our existing markers 1) aren't very easy to understand, and 2) don't map cleanly to the new architecture.
Reviewed By: PeteTheHeat
Differential Revision: D23789156
fbshipit-source-id: 2ed10769e08604e591503a2bc9566aeb1d0563ed
Summary:
Changelog: [Internal]
Construction of Layout will be needed in `TextLayoutManager.measureLines`, pulling it out into separate function prevents code duplication.
Reviewed By: shergin
Differential Revision: D23782905
fbshipit-source-id: 8ab817559ca154716a190ca1012e809c5fa2fd6e
Summary:
This is to prepare for enabling TurboModule on Android. This commit compiles in all the core files (C++) into the ReactAndroid NDK build step. This doesn't yet enable TurboModule by default, just compiling in the infra, just like for iOS.
New shared libs:
* libreact_nativemodule_core.so: The TurboModule Android core
* libreact_nativemodule_manager.so: The TurboModule manager/delegate
To be compatible with `<ReactCommon/` .h include prefix, the files had to move to local `ReactCommon` subdirs.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D23805717
fbshipit-source-id: b41c392a592dd095ae003f7b2a689f4add2c37a9
Summary:
In a previous recent diff we changed Android's Delete mount instruction to *not* recursively delete the tree. This is fine, but because of that, we stopped calling `onDropViewInstance` when views are normally deleted.
Bring back that behaviour.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D23801666
fbshipit-source-id: 54e6b52ab51fff2a45102e37077fe41081499888
Summary:
This diff moves the code of TurboModule Core from ReactCommon/turbomodule to ReactCommon/react/nativemodule
For iOS: Pod spec name stays as "ReactCommon/turbomodule/..." for now, only the source/header location is affected. The target will be renamed/restructured closer to TurboModule rollout.
changelog: [internal] Internal
Reviewed By: RSNara
Differential Revision: D23362253
fbshipit-source-id: c2c8207578e50821c7573255d4319b9051b58a37
Summary:
Because BatchMountItem executes many items, sometimes it's unclear which MountItem causes a crash. Catch and log the exact item.
This shouldn't cause perf regressions because we have a try/catch block in FabricUIManager where execute is called.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D23775500
fbshipit-source-id: c878e085c23d3d3a7ef02a34e5aca57759376aa6
Summary:
There are cases where we Delete+Create a node in the same frame. Practically, the new differ should prevent this, but we don't want to rely on that necessarily.
See comments for further justification on why deleteView can do less work overall.
In reparenting cases, this causes crashes because dropView removes *and deletes* children that shouldn't necessarily be deleted.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D23775453
fbshipit-source-id: c577c5af8c27cfb185d527f0afd8aeb08ee3a5fe
Summary:
Changelog: [internal]
# Problem
Default padding for TextEdit is top: 26, bottom: 29, left: 10, right: 10 however the default values for LayoutMetrics.contentInsets is {0, 0, 0, 0}.
If you try to construct TextEdit with padding 0, Fabric will drop padding update because padding is already 0, 0, 0, 0.
# Fix
To fix this, I added a special case to `Binding::createUpdatePaddingMountItem`, if the mutation is insert, proceed with updating padding.
Reviewed By: JoshuaGross
Differential Revision: D23731498
fbshipit-source-id: 294ab053e562c05aadf6e743fb6bf12285d50307
Summary:
Right now nested Text components are not accessible on Android. This is because we only create a native ReactTextView for the parent component; the styling and touch handling for the child component are handled using spans. In order for TalkBack to announce the link, we need to linkify the text using a ClickableSpan.
This diff adds ReactClickableSpan, which TextLayoutManager uses to linkify a span of text when its corresponding React component has `accessibilityRole="link"`. For example:
<Text>
A paragraph with some
<Text accessible={true} accessibilityRole="link" onPress={onPress} onClick={onClick}>links</Text>
surrounded by other text.
</Text>
With this diff, the child Text component will be announced by TalkBack ('links available') and exposed as an option in the context menu. Clicking on the link in the context menu fires the Text component's onClick, which we're explicitly forwarding to onPress in Text.js (for now - ideally this would probably use a separate event, but that would involve wiring it up in the renderer as well).
ReactClickableSpan also applies text color from React if it exists; this is to override the default Android link styling (teal + underline).
Changelog: [Android][Fixed] Make nested Text components accessible as links
Reviewed By: yungsters, mdvacca
Differential Revision: D23553222
fbshipit-source-id: a962b2833d73ec81047e86cfb41846513c486d87
Summary:
This fixes TextInput measurement caching. Previously we were not setting the correct Spannable in the cache; we needed to additional Spans to it to indicate font size and other attributes.
This brings Fabric closer to how non-Fabric was measuring Spannables for TextInputs (see ReactTextInputShadowNode.java).
This should fix a few crashes and will be most noticeable with dynamically-sized multiline textinputs where the number of lines changes over time.
This also allows us to transmit less data from C++ to Java in the majority of cases.
Changelog: [Internal]
Differential Revision: D23670779
fbshipit-source-id: cf9b8c848b9e0c2619e01766b72b074248466825
Summary:
Remove `enableFabricStartSurfaceWithLayoutMetrics` and treat as `true` always from now on.
Changelog: [Internal]
Differential Revision: D23633198
fbshipit-source-id: 5b7455b87e578ffa97d80746fa901cd2b50d3ea9
Summary:
This log was necessary to get to the bottom of the TurboModule Fb4a eager init crash. It's no longer necessary, plus it's okay for TurboModules to be null, so we'll remove it for now.
Changelog: [Internal]
Differential Revision: D23607208
fbshipit-source-id: 083b00abe6bdefc5986f842cd6f9438f47cce1ce
Summary:
Right now you can scroll a horizontal ScrollView with TalkBack even if you've disabled scrolling in JS, because our HorizontalScrollView component doesn't prevent the accessibility scroll event (this doesn't seem to happen with vertical ScrollViews for some reason...) This diff adds an accessibility delegate to both of the Android ScrollView components to make sure they're not scrollable if scrolling has been disabled in JS.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D23582689
fbshipit-source-id: b670bdb462ab9c963c7125597d60ca97c7d88a9c
Summary:
When we render a text input that already has a text value (<TextInput value="123" />), its selection (cursor) is automatically set to the end of the text. However, when you swipe to focus the text input with TalkBack, Android decides it needs to clear the selection, which moves the cursor back to the beginning of the text input. This is probably not what you want if you're editing some text that's already there. Ideally we would just keep the selection at the end, but I don't know how to prevent this from happening - it seems to be part of how TextView handles the accessibility focus event? So instead I'm just explicitly setting the selection to the end of the text in our handler for accessibility click.
Changelog: [Android][Fixed] Move selection to the end of the text input on accessibility click
Reviewed By: mdvacca
Differential Revision: D23441077
fbshipit-source-id: 16964f5b106637e55a98c6b0ef0f0041e8e6215d
Summary:
After monitoring scuba for a few days, previous fixes(D23301714 D23331828 (https://github.com/facebook/react-native/commit/07a597ad185c8c31ac38bdd4d022b0b880d02859)) don't work as expected.
I managed to test this issue on a Xiaomi device, the crash didn't happen but the there was a popup "Frequetly used email" on top of email edit text:
{F317216473}
Getting rid of the popup probably be the right fix.
For more context see https://github.com/facebook/react-native/issues/27204
Changelog: [Android] - Set caretHidden to true to fix the Xiaomi crash
Reviewed By: mdvacca
Differential Revision: D23451929
fbshipit-source-id: 521931422f3a46a056a9faa4b10fe93cf4732db0
Summary:
This diff optimizes the caching of Spannable objects managed by the TextLayoutManager class.
Previously, these objects were cached using unsing a String representation of the RedableMap (creating this string adds a non trivial cost), this diff improves the caching performance relying on the equals / hashcode methods of the ReadableNativeMap class
I created a MC just to have a killswitch
Motivation: I was analysing another bug and I found this non performant code
changelog: [internal] internal
Reviewed By: shergin
Differential Revision: D23429365
fbshipit-source-id: 59e5ad0b1b95da992ac393aecfe029da68a8df97
Summary:
`#import` is non-standard C++ extension which is AFAIK part of Objective-C. I am surprised that it even compiles on Android.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23407689
fbshipit-source-id: 2861138cbcce33674e118a1ad816e33bbf8f30fe
Summary:
This diff creates the new TransparentImmersiveReactActivity in FB4A, the intention is to help integrate TransparentReactActivity with Fb4A
Changelog: [Deprecated][Android] Deprecated method UIManagerModule.getUIImplementation. This method will not be part of the new architecture of React Native.
Reviewed By: stashuk
Differential Revision: D23324543
fbshipit-source-id: 35395fe410790a9611a4637361b888678eb0a836
Summary:
We have password components which allow visibility to be toggled by
setting both the keyboardType and secureTextEntry props. The order in
which those updates are executed is determined by iterating a NativeMap
of props, and the iteration order of a NativeMap is implementation
dependent.
With libc++ as our STL, we observe that setSecureTextEntry is called
before setKeyboardType. This results in the following sequence of input
type flag settings when toggling the component to visible and then back
to hidden:
* The field starts out with TYPE_TEXT_VARIATION_PASSWORD (0x80).
* When we toggle to visible, setSecureTextEntry is called with password
being false, which clears TYPE_TEXT_VARIATION_PASSWORD.
setKeyboardType is then called with the visible-password keyboard
type, which sets TYPE_TEXT_VARIATION_VISIBLE_PASSWORD (0x90).
* When we toggle back to hidden, setSecureTextEntry is called with
password being true, which sets TYPE_TEXT_VARIATION_PASSWORD but
doesn't clear TYPE_TEXT_VARIATION_VISIBLE_PASSWORD. setKeyboardType is
then called with the default keyboard type and additionally sets
TYPE_CLASS_TEXT, but TYPE_TEXT_VARIATION_VISIBLE_PASSWORD remains and
the password field remains visible.
The fix is to clear TYPE_TEXT_VARIATION_VISIBLE_PASSWORD when
setSecureTextEntry is called with password being true, to ensure the
password gets hidden.
Changelog:
[Android][Fixed] - Fix secure text entry setting to always hide text
Reviewed By: shergin
Differential Revision: D23399174
fbshipit-source-id: a81deec702e768672e2103b76ab50ec728dac229
Summary:
Long term fix in native for Error: android_crash:java.lang.NullPointerException:android.widget.Editor$SelectionModifierCursorController.access$300
For more detail please see T68183343 D23301714
Changelog:
[Android][Changed] - Fix Xiaomi TextInput crash in native
Reviewed By: mdvacca
Differential Revision: D23331828
fbshipit-source-id: 914f2d431772f49711b940d47a2b3ef57ab82cdc
Summary:
Make sure to check incoming state values before calling SetState, or we call back and forth forever.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D23389355
fbshipit-source-id: 9cf6110cf654fe93f555a6fbfd9b20f112214e0a
Summary:
See title.
This feature makes LayoutAnimations less stable and isn't needed generally; will be deleted soon.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23382973
fbshipit-source-id: f633f482d463b3ee3e4625b30544a33cd6e36119
Summary:
In some cases (BottomSheet?) the parent/ViewManager removes all children of the View before Fabric gets a chance to remove the children.
Apparently prior to D23368229 (https://github.com/facebook/react-native/commit/d344fb4e29a827d1e7d233672a3efe3b2b981a8a) (landed just today!) this sequence of operations happened and just noop'ed. Since we've been doing that happily as long as Fabric
has existed, we'll keep doing that for now.
I suspect that on *some* versions of Android this crashes and others it doesn't, based on logviews and my inability to repro certain crashes.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23387044
fbshipit-source-id: 88a46191adef4f6816bd7babd9103d103ddcef33
Summary:
If we try to delete a view and find the wrong one, when we crash, try to log the *actual* index of the view in question.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23368229
fbshipit-source-id: 7f9835fd07cfe4924d05c7e37b42b9bcdffff4a9
Summary:
Now we store a revision number of a Shadow Tree that leads to a transaction for which the concrete instance of MountingTelemetry corresponds. This is useful to understand how many actual transactions were skipped during a mounting phase (a mounting transaction does not directly correspond to a commit operation).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D23364663
fbshipit-source-id: 32b86bcdfc1ae97d8fff3b97a8615cc5a5b4d4a9
Summary:
Previously this was crashing only in debug, but that's too noisy and isn't giving us any value for now.
Changelog: [Internal]
Differential Revision: D23338800
fbshipit-source-id: bf1535cdda231ccf30af6d00509eec1499a552a1
Summary:
Simplifying the StopSurface flow. Before we would still attempt to execute MountItems, but only the "Delete" operations. This was... well, frankly, overcomplicated. Instead we can just ignore all future MountInstructions for that Surface and delete all views recursively from the root.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23338752
fbshipit-source-id: 6e7ab29ad85572782bfc6a39845a8a619f001559
Summary:
Microsoft’s RN for macOS fork supports the Hermes engine nowadays https://github.com/microsoft/react-native-macos/pull/473. As a longer term work item, we’ve started moving bits that are not invasive for iOS but _are_ a maintenance burden on us—mostly when merging—upstream. Seeing as this one is a recent addition, it seemed like a good candidate to start with.
As to the actual changes, these include:
* Sharing Android’s Hermes executor with the objc side of the codebase.
* Adding a CocoaPods subspec to build the Hermes inspector source and its dependencies (`Folly/Futures`, `libevent`).
* Adding the bits to the Xcode build phase script that creates the JS bundle for release builds to compile Hermes bytecode and source-maps…
* …coincidentally it turns out that the Xcode build phase script did _not_ by default output source-maps for iOS, which is now fixed too.
All of the Hermes bits are automatically enabled, on macOS, when providing the `hermes-engine-darwin` [npm package](https://www.npmjs.com/package/hermes-engine-darwin) and enabling the Hermes pods.
## Changelog
[General] [Added] - Upstream RN macOS Hermes integration bits
Pull Request resolved: https://github.com/facebook/react-native/pull/29748
Test Plan:
Building RNTester for iOS and Android still works as before.
To test the actual changes themselves, you’ll have to use the macOS target in RNTester in the macOS fork, or create a new application from `master`:
<img width="812" alt="Screenshot 2020-08-18 at 16 55 06" src="https://user-images.githubusercontent.com/2320/90547606-160f6480-e18c-11ea-9a98-edbbaa755800.png">
Reviewed By: TheSavior
Differential Revision: D23304618
Pulled By: fkgozali
fbshipit-source-id: 4ef0e0f60d909f3c59f9cfc87c667189df656a3b
Summary:
For some reason, these got out of sync again.
This diff modifies the Java Codegen to sort all the methods.
build-break
overriding_review_checks_triggers_an_audit_and_retroactive_review
Differential Revision:
D23363410
Oncall Short Name: fbandroid_sheriff
Ninja: master broken
fbshipit-source-id: 257d85f92017528e64ced31bc7be011acb333186
Summary:
I'm removing an ill-informed "optimization" that resulted in some StateUpdates being dropped. For some components (including TextInput) we rely on State updates to request a layout from the ShadowNode layer.
In the past we were performing an optimization that didn't update the View layer if the data contained in the State container is identical, but in the case of TextInput and other components, we simply pass an opaque
object with no meaningful data to trigger the layouts. In those cases, it could cause a permanent rift between the View layer's StateWrapper and the most recent state object from the C++ perspective.
In the case of TextInput this didn't cause tangible bugs because you can always update state using an out-of-date State object, but it's better this way anyway.
The other issue is that for some components, we want to know when there's a State update from the Cxx layer. This optimization broke certain logic in those components.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23306222
fbshipit-source-id: 8ef83149b814de50776674b5fd22406be1db14ba
Summary:
# What is this?
For a very long time, we've discussed the possibility of detecting Node Reparenting in the Fabric Differ. Practically, from the developer perspective, ReactJS and React Native do not allow reparenting: nodes cannot be reparented, only deleted and then recreated with entirely new tags.
However, Fabric introduced the idea of View Flattening where views deemed unnecessary would be removed from the View hierarchy entirely. This is great and improves memory usage, except for one issue: if a View becomes unflattened, or becomes flattened, the entire tree underneath it must be rebuilt.
In a past diff we introduced a mechanism to detect sibling reordering cleverly, and produce a minimal instruction set. This diff is very similar: we know the invariants around flattening and unflattening of views and we take advantage of them to produce an optimal set of instructions efficiently.
# What's different from previous attempts?
No global maps! Those are slow!
This seems to work and (hopefully) might even improve performance, since way less work is being done on the UI thread in cases when views are (un)flattened.
This *only* does extra work when flattening/unflattening happens, which gives product engineers a little more control over perf.
# So, how's it work?
This algorithm is intuitively simple (I think) but tricky to pull off, because there are lots of edge-cases.
In short: In the past, that information was hidden from the Differ: the differ didn't know if views were being reparented, it would see them
as entirely new views or as views being deleted if a View was flattened or unflattened. We very subtly change the information given to the differ:
all nodes are visible to the differ, but marked as Flattened or Unflattened. Thus, when the differ compares two nodes in the "old" and "new" tree,
it can tell not just if there are updates to the node but if it has been unflattened or flattened as well.
For example, take this tree, where * indicates that a View is flattened:
```
A
+
+----+---+
B* X
+ +
| |
+---+--+ +
E F Y
```
When the Differ asks for the children of A, in the past it would get a list `[E, F, X]`. That is, B* and X are both its children, but since B is flattened, it is omitted entirely from the list and
its children are substituted.
Now, when the Differ asks for the children of A, we give it this list instead: `[B*, E, F, X]`. That is: we give it a list which includes B, but B is marked as flattened.
Another wrinkle: A node `X` could have its children flattened, but still be a concrete view: so flattening/unflattening is a different operation from making a view "concrete" or "unconcrete", which can change independently of flattening.
There is one additional wrinkle: because of zIndex/stacking order, the children of `B` might not actually appear after `B` in the list. Depending on zIndex, a tree that looks like this:
```
A
+
+------+------+
B* C*
+ +
| |
+--+--+ +--+--+
D E F G
```
Could actually be linearized as: `[D G B* F C* E]` (as an extreme example; but basically all permutations as possible).
This is the reason, and the *only* reason that the inner Flattener/Unflattener
## The cases we need to handle
There are 7 cases/edge-cases of flattening and unflattening that we need to handle. Practically, all cases of reordering + flattening/unflattening, and taking recursive cases into account:
1. View A and A' (A in the old tree, A' in the new tree) are matched in the differ, and A* has been flattened or unflattened. These two cases are the easiest to handle.
2. View A' has been reordered with its siblings, and has been flattened or unflattened. These cases are slightly trickier to handle.
3. While flattening or unflattening, we encounter a child that has also been unflattened or flattened. So we need to handle four cases here in total: Flatten-Flatten, Flatten-Unflatten, Unflatten-Flatten, and Unflatten-Unflatten.
Other things to think about, also covered above:
1. Ordering. Views can be reordered and flattened/unflattened at the same time.
2. zIndex ordering: children in a certain order from the ShadowNode perspective may be stacked differently from a View perspective. We use the zIndex ordering for everything in the differ, and this prevents us from performing certain optimizations (see above: we cannot assume that children come after their parent in a list; they may come before, may be interwoven with children from other parents, etc).
# Perf Implications?
Practically, there should be very little negative overhead. There is some overhead in actually performing a flattening/unflattening operation, but... not much more than before. We don't use global maps, so the cost of flattening/unflattening is basically `O(number of nodes reparented)` - note that that's direct nodes reparented, *not* descendants.
tl;dr the perf hit should be similar to reordering, which is non-zero, but close to zero, and zero-cost for any diff operations on parts of the tree that don't involve flattening/unflattening. AFAICT this is very close to an ideal solution for that reason (but I wish it was simpler overall).
# In Summary?
I hope this works out and I think it could improve a number of things downstream: perf, LayoutAnimations, Bindings, certain crashes because of platform assumptions about mutations, etc.
Is it worth it? This new implementation is substantially harder to reason about, harder to read, and harder to understand. This is an important consideration. All I can say there is that I trust the test suite I've been using, but
the decreased readability is a big negative. Hopefully we can improve this in the future.
The rest is fiddly implementation details that I sincerely hope can be improved and simplified in the future.
# Followups?
The part that makes this algorithm the most expensive is that because of zIndex ordering, we cannot assume that children are linearized after their parents and so we rely more heavily on maps for the flattening/unflattening. Our TinyMap implementation should make these `find` operations fast enough unless trees' children are constantly being reordered, but it's still worth thinking of ways to make this even faster.
Changelog: [Internal]
Reviewed By: shergin, mdvacca
Differential Revision: D23259341
fbshipit-source-id: 35d9b90caf262d601a31996ea2cb37e329c61ffc
Summary:
Simplify the TextInput measurement mechanism.
Now, data only flows from JS->C++->Java and from Java->JS. C++ passes along AttributedStrings from JS if JS updates, and otherwise Java maintains the only source of truth.
Previously we tried to keep all three in sync. This was complicated, slow, and even lead to some crashes.
This feels a bit hacky but I believe it's the simplest way to achieve this short-term. Ideally, we would use something like `AttributedStringBox` and pass that to State from Java,
but currently everything passed through the State system from Java must be serializable as `folly::dynamic`. So, instead, we just cache one Spannable per TextInput component and
use ReactTag as the cache identifier for lookup.
An interesting side-effect is that `measure` could race with TextInput updates, but the race condition favors measuring the latest text, not outdated values.
Followups:
- Can we do this without copying the EditText Spannable on every keystroke? Maybe this approach is too aggressive, but I don't want a background thread measuring a Spannable as it's being mutated.
- Do we need to support measuring Attachments?
- How can we clean up this API? It should work for now, but feels a little hacky.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23290230
fbshipit-source-id: 832d2f397d30dfb17b77958af970d9c52a37e88b
Summary:
In MountingManager.java in Fabric, if we drop a view with any attached views, we also drop all children. If verbose logging is turned on, log all instances of that happening.
This has no impact unless you switch the flag on manually in debug mode.
Changelog: [Internal]
Differential Revision: D23257749
fbshipit-source-id: fce4476aa47cc1b7137cd9fd2fd0241af1593288