Skip to content

Commit e364c7f

Browse files
NickGerlemanfacebook-github-bot
authored andcommitted
Fix incorrect hit testing on text when layout reused (#52692)
Summary: D77341994 added a global cache for Facsimile layouts, where we will reuse an existing `android.text.Layout`, and `Spannable`, if one already existed for a same AttributedString, under same layout constraints. An error here, is that we consider a layout reusable, even when shadow views are different, which means there may be different react tags in the underlying AttributedString. This leaks into the layout itself, and means that if a layout is reused, we can hit test against a stale/incorrect react tag. The solution to allow reuse here, is to avoid embedding react tag directly into the `android.text.Layout` structure. Instead, we replace references to react tags, with a fragment index, and embed the list of fragment indices in each PreparedLayout. We can hide this "cleverness" within the boundary of `TextLayoutManager`, such that an invalid `PreparedLayout` is never allowed to escape. Changelog: [Internal] Differential Revision: D78516079
1 parent 862e8c7 commit e364c7f

File tree

8 files changed

+204
-40
lines changed

8 files changed

+204
-40
lines changed

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,18 @@ public PreparedLayout prepareTextLayout(
675675
: null);
676676
}
677677

678+
@AnyThread
679+
@ThreadConfined(ANY)
680+
@UnstableReactNativeAPI
681+
public PreparedLayout reusePreparedLayoutWithNewReactTags(
682+
PreparedLayout preparedLayout, int[] reactTags) {
683+
return new PreparedLayout(
684+
preparedLayout.getLayout(),
685+
preparedLayout.getMaximumNumberOfLines(),
686+
preparedLayout.getVerticalOffset(),
687+
reactTags);
688+
}
689+
678690
@AnyThread
679691
@ThreadConfined(ANY)
680692
@UnstableReactNativeAPI

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayout.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,6 @@ import com.facebook.proguard.annotations.DoNotStrip
1818
internal class PreparedLayout(
1919
val layout: Layout,
2020
val maximumNumberOfLines: Int,
21-
val verticalOffset: Float
21+
val verticalOffset: Float,
22+
val reactTags: IntArray,
2223
)

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayoutTextView.kt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import com.facebook.proguard.annotations.DoNotStrip
2727
import com.facebook.react.uimanager.BackgroundStyleApplicator
2828
import com.facebook.react.uimanager.ReactCompoundView
2929
import com.facebook.react.uimanager.style.Overflow
30-
import com.facebook.react.views.text.internal.span.ReactTagSpan
30+
import com.facebook.react.views.text.internal.span.ReactFragmentIndexSpan
3131
import kotlin.collections.ArrayList
3232
import kotlin.math.roundToInt
3333

@@ -310,8 +310,9 @@ internal class PreparedLayoutTextView(context: Context) : ViewGroup(context), Re
310310
override fun hasOverlappingRendering(): Boolean = false
311311

312312
override fun reactTagForTouch(touchX: Float, touchY: Float): Int =
313-
getSpanInCoords(touchX.roundToInt(), touchY.roundToInt(), ReactTagSpan::class.java)?.reactTag
314-
?: id
313+
getSpanInCoords(touchX.roundToInt(), touchY.roundToInt(), ReactFragmentIndexSpan::class.java)
314+
?.fragmentIndex
315+
?.let { preparedLayout?.reactTags[it] } ?: id
315316

316317
@RequiresApi(api = Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
317318
private object Api34Utils {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ import com.facebook.react.views.text.internal.span.ReactAbsoluteSizeSpan
4242
import com.facebook.react.views.text.internal.span.ReactBackgroundColorSpan
4343
import com.facebook.react.views.text.internal.span.ReactClickableSpan
4444
import com.facebook.react.views.text.internal.span.ReactForegroundColorSpan
45+
import com.facebook.react.views.text.internal.span.ReactFragmentIndexSpan
46+
import com.facebook.react.views.text.internal.span.ReactLinkSpan
4547
import com.facebook.react.views.text.internal.span.ReactOpacitySpan
4648
import com.facebook.react.views.text.internal.span.ReactStrikethroughSpan
4749
import com.facebook.react.views.text.internal.span.ReactTagSpan
@@ -218,7 +220,8 @@ internal object TextLayoutManager {
218220
context: Context,
219221
fragments: MapBuffer,
220222
sb: SpannableStringBuilder,
221-
ops: MutableList<SetSpanOperation>
223+
ops: MutableList<SetSpanOperation>,
224+
outputReactTags: IntArray?
222225
) {
223226
for (i in 0 until fragments.count) {
224227
val fragment = fragments.getMapBuffer(i)
@@ -249,7 +252,11 @@ internal object TextLayoutManager {
249252
(textAttributes.mAccessibilityRole ==
250253
ReactAccessibilityDelegate.AccessibilityRole.LINK)
251254
if (roleIsLink) {
252-
ops.add(SetSpanOperation(start, end, ReactClickableSpan(reactTag)))
255+
if (ReactNativeFeatureFlags.enablePreparedTextLayout()) {
256+
ops.add(SetSpanOperation(start, end, ReactLinkSpan(i)))
257+
} else {
258+
ops.add(SetSpanOperation(start, end, ReactClickableSpan(reactTag)))
259+
}
253260
}
254261
if (textAttributes.mIsColorSet) {
255262
ops.add(SetSpanOperation(start, end, ReactForegroundColorSpan(textAttributes.mColor)))
@@ -307,7 +314,14 @@ internal object TextLayoutManager {
307314
start, end, CustomLineHeightSpan(textAttributes.effectiveLineHeight)))
308315
}
309316

310-
ops.add(SetSpanOperation(start, end, ReactTagSpan(reactTag)))
317+
if (ReactNativeFeatureFlags.enablePreparedTextLayout()) {
318+
ops.add(SetSpanOperation(start, end, ReactFragmentIndexSpan(i)))
319+
if (outputReactTags != null) {
320+
outputReactTags[i] = reactTag
321+
}
322+
} else {
323+
ops.add(SetSpanOperation(start, end, ReactTagSpan(reactTag)))
324+
}
311325
}
312326
}
313327
}
@@ -323,7 +337,8 @@ internal object TextLayoutManager {
323337

324338
private fun buildSpannableFromFragmentsOptimized(
325339
context: Context,
326-
fragments: MapBuffer
340+
fragments: MapBuffer,
341+
outputReactTags: IntArray?
327342
): Spannable {
328343
val text = StringBuilder()
329344
val parsedFragments = ArrayList<FragmentAttributes>(fragments.count)
@@ -363,7 +378,7 @@ internal object TextLayoutManager {
363378
val spannable = SpannableString(text)
364379

365380
var start = 0
366-
for (fragment in parsedFragments) {
381+
for ((i, fragment) in parsedFragments.withIndex()) {
367382
val end = start + fragment.length
368383
val spanFlags =
369384
if (start == 0) Spannable.SPAN_INCLUSIVE_INCLUSIVE else Spannable.SPAN_EXCLUSIVE_INCLUSIVE
@@ -386,7 +401,11 @@ internal object TextLayoutManager {
386401
ReactAccessibilityDelegate.AccessibilityRole.LINK)
387402

388403
if (roleIsLink) {
389-
spannable.setSpan(ReactClickableSpan(fragment.reactTag), start, end, spanFlags)
404+
if (ReactNativeFeatureFlags.enablePreparedTextLayout()) {
405+
spannable.setSpan(ReactLinkSpan(i), start, end, spanFlags)
406+
} else {
407+
spannable.setSpan(ReactClickableSpan(fragment.reactTag), start, end, spanFlags)
408+
}
390409
}
391410

392411
if (fragment.props.isColorSet) {
@@ -453,7 +472,14 @@ internal object TextLayoutManager {
453472
CustomLineHeightSpan(fragment.props.effectiveLineHeight), start, end, spanFlags)
454473
}
455474

456-
spannable.setSpan(ReactTagSpan(fragment.reactTag), start, end, spanFlags)
475+
if (ReactNativeFeatureFlags.enablePreparedTextLayout()) {
476+
spannable.setSpan(ReactFragmentIndexSpan(i), start, end, spanFlags)
477+
if (outputReactTags != null) {
478+
outputReactTags[i] = fragment.reactTag
479+
}
480+
} else {
481+
spannable.setSpan(ReactTagSpan(fragment.reactTag), start, end, spanFlags)
482+
}
457483
}
458484

459485
start = end
@@ -474,21 +500,23 @@ internal object TextLayoutManager {
474500
} else {
475501
text =
476502
createSpannableFromAttributedString(
477-
context, attributedString, reactTextViewManagerCallback)
503+
context,
504+
attributedString.getMapBuffer(AS_KEY_FRAGMENTS),
505+
reactTextViewManagerCallback,
506+
null)
478507
}
479508

480509
return text
481510
}
482511

483512
private fun createSpannableFromAttributedString(
484513
context: Context,
485-
attributedString: MapBuffer,
486-
reactTextViewManagerCallback: ReactTextViewManagerCallback?
514+
fragments: MapBuffer,
515+
reactTextViewManagerCallback: ReactTextViewManagerCallback?,
516+
outputReactTags: IntArray?
487517
): Spannable {
488518
if (ReactNativeFeatureFlags.enableAndroidTextMeasurementOptimizations()) {
489-
val spannable =
490-
buildSpannableFromFragmentsOptimized(
491-
context, attributedString.getMapBuffer(AS_KEY_FRAGMENTS))
519+
val spannable = buildSpannableFromFragmentsOptimized(context, fragments, outputReactTags)
492520

493521
reactTextViewManagerCallback?.onPostProcessSpannable(spannable)
494522
return spannable
@@ -500,7 +528,7 @@ internal object TextLayoutManager {
500528
// a new spannable will be wiped out
501529
val ops: MutableList<SetSpanOperation> = ArrayList()
502530

503-
buildSpannableFromFragments(context, attributedString.getMapBuffer(AS_KEY_FRAGMENTS), sb, ops)
531+
buildSpannableFromFragments(context, fragments, sb, ops, outputReactTags)
504532

505533
// TODO T31905686: add support for inline Images
506534
// While setting the Spans on the final text, we also check whether any of them are images.
@@ -756,7 +784,11 @@ internal object TextLayoutManager {
756784
heightYogaMeasureMode: YogaMeasureMode,
757785
reactTextViewManagerCallback: ReactTextViewManagerCallback?
758786
): PreparedLayout {
759-
val text = getOrCreateSpannableForText(context, attributedString, reactTextViewManagerCallback)
787+
val fragments = attributedString.getMapBuffer(AS_KEY_FRAGMENTS)
788+
val reactTags = IntArray(fragments.count)
789+
val text =
790+
createSpannableFromAttributedString(
791+
context, fragments, reactTextViewManagerCallback, reactTags)
760792
val baseTextAttributes =
761793
TextAttributeProps.fromMapBuffer(attributedString.getMapBuffer(AS_KEY_BASE_ATTRIBUTES))
762794
val layout =
@@ -779,7 +811,7 @@ internal object TextLayoutManager {
779811
getVerticalOffset(
780812
layout, paragraphAttributes, height, heightYogaMeasureMode, maximumNumberOfLines)
781813

782-
return PreparedLayout(layout, maximumNumberOfLines, verticalOffset)
814+
return PreparedLayout(layout, maximumNumberOfLines, verticalOffset, reactTags)
783815
}
784816

785817
@JvmStatic
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.text.internal.span
9+
10+
/**
11+
* Maps a section of the text to the index of the AttributedString fragment originally used to
12+
* create it.
13+
*/
14+
internal class ReactFragmentIndexSpan(val fragmentIndex: Int) : ReactSpan
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.text.internal.span
9+
10+
import android.text.TextPaint
11+
import android.text.style.ClickableSpan
12+
import android.view.View
13+
import com.facebook.react.bridge.ReactContext
14+
import com.facebook.react.uimanager.UIManagerHelper
15+
import com.facebook.react.views.text.PreparedLayoutTextView
16+
import com.facebook.react.views.text.TextLayoutManager
17+
import com.facebook.react.views.view.ViewGroupClickEvent
18+
19+
/**
20+
* This class is used in [TextLayoutManager] to linkify and style a span of text with
21+
* accessibilityRole="link". This is needed to make nested Text components accessible.
22+
*
23+
* For example, if your React component looks like this:
24+
* ```js
25+
* <Text>
26+
* Some text with
27+
* <Text onPress={onPress} accessible={true} accessibilityRole="link">a link</Text>
28+
* in the middle.
29+
* </Text>
30+
* ```
31+
*/
32+
internal class ReactLinkSpan(val fragmentIndex: Int) : ClickableSpan(), ReactSpan {
33+
override fun onClick(view: View) {
34+
val context = view.context as ReactContext
35+
val textView = view as? PreparedLayoutTextView ?: return
36+
val preparedLayout = textView.preparedLayout ?: return
37+
val reactTag = preparedLayout.reactTags[fragmentIndex]
38+
val eventDispatcher = UIManagerHelper.getEventDispatcherForReactTag(context, reactTag)
39+
eventDispatcher?.dispatchEvent(
40+
ViewGroupClickEvent(UIManagerHelper.getSurfaceId(context), reactTag))
41+
}
42+
43+
override fun updateDrawState(ds: TextPaint) {
44+
// no super call so we don't change the link color or add an underline by default, as the
45+
// superclass does.
46+
}
47+
}

packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.cpp

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,14 +310,19 @@ TextLayoutManager::PreparedLayout TextLayoutManager::prepareLayout(
310310
jfloat,
311311
jfloat)>("prepareTextLayout");
312312

313-
return preparedTextCache_.get(
313+
static auto reusePreparedLayoutWithNewReactTags =
314+
jni::findClassStatic("com/facebook/react/fabric/FabricUIManager")
315+
->getMethod<JPreparedLayout::javaobject(
316+
JPreparedLayout::javaobject, jintArray)>(
317+
"reusePreparedLayoutWithNewReactTags");
318+
319+
const auto [key, preparedText] = preparedTextCache_.getWithKey(
314320
{.attributedString = attributedString,
315321
.paragraphAttributes = paragraphAttributes,
316322
.layoutConstraints = layoutConstraints},
317-
[&] {
323+
[&]() {
318324
const auto& fabricUIManager =
319325
contextContainer_->at<jni::global_ref<jobject>>("FabricUIManager");
320-
321326
auto attributedStringMB = JReadableMapBuffer::createWithContents(
322327
toMapBuffer(attributedString));
323328
auto paragraphAttributesMB = JReadableMapBuffer::createWithContents(
@@ -336,6 +341,39 @@ TextLayoutManager::PreparedLayout TextLayoutManager::prepareLayout(
336341
minimumSize.height,
337342
maximumSize.height))};
338343
});
344+
345+
// PreparedTextCacheKey allows equality of layouts which are the same
346+
// display-wise, but ShadowView fragments (and thus react tags) may have
347+
// changed.
348+
const auto& fragments = attributedString.getFragments();
349+
const auto& cacheKeyFragments = key->attributedString.getFragments();
350+
bool needsNewReactTags = [&] {
351+
for (size_t i = 0; i < fragments.size(); i++) {
352+
if (fragments[i].parentShadowView.tag !=
353+
cacheKeyFragments[i].parentShadowView.tag) {
354+
return true;
355+
}
356+
}
357+
return false;
358+
}();
359+
360+
if (needsNewReactTags) {
361+
std::vector<int> reactTags(fragments.size());
362+
for (size_t i = 0; i < reactTags.size(); i++) {
363+
reactTags[i] = fragments[i].parentShadowView.tag;
364+
}
365+
366+
auto javaReactTags = jni::JArrayInt::newArray(fragments.size());
367+
javaReactTags->setRegion(
368+
0, static_cast<jsize>(reactTags.size()), reactTags.data());
369+
370+
const auto& fabricUIManager =
371+
contextContainer_->at<jni::global_ref<jobject>>("FabricUIManager");
372+
return PreparedLayout{jni::make_global(reusePreparedLayoutWithNewReactTags(
373+
fabricUIManager, preparedText->get(), javaReactTags.get()))};
374+
} else {
375+
return PreparedLayout{*preparedText};
376+
}
339377
}
340378

341379
TextMeasurement TextLayoutManager::measurePreparedLayout(

packages/react-native/ReactCommon/react/utils/SimpleThreadSafeCache.h

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,24 +38,20 @@ class SimpleThreadSafeCache {
3838
*/
3939
ValueT get(const KeyT& key, CacheGeneratorFunction<ValueT> auto generator)
4040
const {
41-
std::lock_guard<std::mutex> lock(mutex_);
42-
43-
if (auto it = map_.find(key); it != map_.end()) {
44-
// Move accessed item to front of list
45-
list_.splice(list_.begin(), list_, it->second);
46-
return it->second->second;
47-
}
41+
return getMapIterator(key, std::move(generator))->second->second;
42+
}
4843

49-
auto value = generator();
50-
// Add new value to front of list and map
51-
list_.emplace_front(key, value);
52-
map_[key] = list_.begin();
53-
if (list_.size() > maxSize_) {
54-
// Evict least recently used item (back of list)
55-
map_.erase(list_.back().first);
56-
list_.pop_back();
57-
}
58-
return value;
44+
/*
45+
* Returns pointers to both the key and value from the map with a given key.
46+
* If the value wasn't found in the cache, constructs the value using given
47+
* generator function, stores it inside a cache and returns it.
48+
* Can be called from any thread.
49+
*/
50+
std::pair<const KeyT*, const ValueT*> getWithKey(
51+
const KeyT& key,
52+
CacheGeneratorFunction<ValueT> auto generator) const {
53+
auto it = getMapIterator(key, std::move(generator));
54+
return std::make_pair(&it->first, &it->second->second);
5955
}
6056

6157
/*
@@ -79,6 +75,29 @@ class SimpleThreadSafeCache {
7975
using EntryT = std::pair<KeyT, ValueT>;
8076
using iterator = typename std::list<EntryT>::iterator;
8177

78+
auto getMapIterator(
79+
const KeyT& key,
80+
CacheGeneratorFunction<ValueT> auto generator) const {
81+
std::lock_guard<std::mutex> lock(mutex_);
82+
83+
if (auto it = map_.find(key); it != map_.end()) {
84+
// Move accessed item to front of list
85+
list_.splice(list_.begin(), list_, it->second);
86+
return it;
87+
}
88+
89+
auto value = generator();
90+
// Add new value to front of list and map
91+
list_.emplace_front(key, value);
92+
auto [it, _] = map_.insert_or_assign(key, list_.begin());
93+
if (list_.size() > maxSize_) {
94+
// Evict least recently used item (back of list)
95+
map_.erase(list_.back().first);
96+
list_.pop_back();
97+
}
98+
return it;
99+
}
100+
82101
size_t maxSize_;
83102
mutable std::mutex mutex_;
84103
mutable std::list<EntryT> list_;

0 commit comments

Comments
 (0)