Fix incorrect hit testing on text when layout reused (#52692)

Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/52692 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] Reviewed By: joevilches Differential Revision: D78516079 fbshipit-source-id: 2d9fe9d80f60e6d7e7e40080a0817a08b51c3153
2025-11-01 09:14:26 +00:00 · 2025-07-18 15:19:58 -07:00
parent 7ea0ef7a90
commit ea394f6229
8 changed files with 204 additions and 40 deletions
@@ -675,6 +675,18 @@ public class FabricUIManager
            : null);
  }

+  @AnyThread
+  @ThreadConfined(ANY)
+  @UnstableReactNativeAPI
+  public PreparedLayout reusePreparedLayoutWithNewReactTags(
+      PreparedLayout preparedLayout, int[] reactTags) {
+    return new PreparedLayout(
+        preparedLayout.getLayout(),
+        preparedLayout.getMaximumNumberOfLines(),
+        preparedLayout.getVerticalOffset(),
+        reactTags);
+  }
+
  @AnyThread
  @ThreadConfined(ANY)
  @UnstableReactNativeAPI
@@ -18,5 +18,6 @@ import com.facebook.proguard.annotations.DoNotStrip
 internal class PreparedLayout(
    val layout: Layout,
    val maximumNumberOfLines: Int,
-    val verticalOffset: Float
+    val verticalOffset: Float,
+    val reactTags: IntArray,
 )
@@ -27,7 +27,7 @@ import com.facebook.proguard.annotations.DoNotStrip
 import com.facebook.react.uimanager.BackgroundStyleApplicator
 import com.facebook.react.uimanager.ReactCompoundView
 import com.facebook.react.uimanager.style.Overflow
-import com.facebook.react.views.text.internal.span.ReactTagSpan
+import com.facebook.react.views.text.internal.span.ReactFragmentIndexSpan
 import kotlin.collections.ArrayList
 import kotlin.math.roundToInt

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

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

  @RequiresApi(api = Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
  private object Api34Utils {
@@ -42,6 +42,8 @@ import com.facebook.react.views.text.internal.span.ReactAbsoluteSizeSpan
 import com.facebook.react.views.text.internal.span.ReactBackgroundColorSpan
 import com.facebook.react.views.text.internal.span.ReactClickableSpan
 import com.facebook.react.views.text.internal.span.ReactForegroundColorSpan
+import com.facebook.react.views.text.internal.span.ReactFragmentIndexSpan
+import com.facebook.react.views.text.internal.span.ReactLinkSpan
 import com.facebook.react.views.text.internal.span.ReactOpacitySpan
 import com.facebook.react.views.text.internal.span.ReactStrikethroughSpan
 import com.facebook.react.views.text.internal.span.ReactTagSpan
@@ -218,7 +220,8 @@ internal object TextLayoutManager {
      context: Context,
      fragments: MapBuffer,
      sb: SpannableStringBuilder,
-      ops: MutableList<SetSpanOperation>
+      ops: MutableList<SetSpanOperation>,
+      outputReactTags: IntArray?
  ) {
    for (i in 0 until fragments.count) {
      val fragment = fragments.getMapBuffer(i)
@@ -249,7 +252,11 @@ internal object TextLayoutManager {
                (textAttributes.mAccessibilityRole ==
                    ReactAccessibilityDelegate.AccessibilityRole.LINK)
        if (roleIsLink) {
-          ops.add(SetSpanOperation(start, end, ReactClickableSpan(reactTag)))
+          if (ReactNativeFeatureFlags.enablePreparedTextLayout()) {
+            ops.add(SetSpanOperation(start, end, ReactLinkSpan(i)))
+          } else {
+            ops.add(SetSpanOperation(start, end, ReactClickableSpan(reactTag)))
+          }
        }
        if (textAttributes.mIsColorSet) {
          ops.add(SetSpanOperation(start, end, ReactForegroundColorSpan(textAttributes.mColor)))
@@ -307,7 +314,14 @@ internal object TextLayoutManager {
                  start, end, CustomLineHeightSpan(textAttributes.effectiveLineHeight)))
        }

-        ops.add(SetSpanOperation(start, end, ReactTagSpan(reactTag)))
+        if (ReactNativeFeatureFlags.enablePreparedTextLayout()) {
+          ops.add(SetSpanOperation(start, end, ReactFragmentIndexSpan(i)))
+          if (outputReactTags != null) {
+            outputReactTags[i] = reactTag
+          }
+        } else {
+          ops.add(SetSpanOperation(start, end, ReactTagSpan(reactTag)))
+        }
      }
    }
  }
@@ -323,7 +337,8 @@ internal object TextLayoutManager {

  private fun buildSpannableFromFragmentsOptimized(
      context: Context,
-      fragments: MapBuffer
+      fragments: MapBuffer,
+      outputReactTags: IntArray?
  ): Spannable {
    val text = StringBuilder()
    val parsedFragments = ArrayList<FragmentAttributes>(fragments.count)
@@ -363,7 +378,7 @@ internal object TextLayoutManager {
    val spannable = SpannableString(text)

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

        if (roleIsLink) {
-          spannable.setSpan(ReactClickableSpan(fragment.reactTag), start, end, spanFlags)
+          if (ReactNativeFeatureFlags.enablePreparedTextLayout()) {
+            spannable.setSpan(ReactLinkSpan(i), start, end, spanFlags)
+          } else {
+            spannable.setSpan(ReactClickableSpan(fragment.reactTag), start, end, spanFlags)
+          }
        }

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

-        spannable.setSpan(ReactTagSpan(fragment.reactTag), start, end, spanFlags)
+        if (ReactNativeFeatureFlags.enablePreparedTextLayout()) {
+          spannable.setSpan(ReactFragmentIndexSpan(i), start, end, spanFlags)
+          if (outputReactTags != null) {
+            outputReactTags[i] = fragment.reactTag
+          }
+        } else {
+          spannable.setSpan(ReactTagSpan(fragment.reactTag), start, end, spanFlags)
+        }
      }

      start = end
@@ -474,7 +500,10 @@ internal object TextLayoutManager {
    } else {
      text =
          createSpannableFromAttributedString(
-              context, attributedString, reactTextViewManagerCallback)
+              context,
+              attributedString.getMapBuffer(AS_KEY_FRAGMENTS),
+              reactTextViewManagerCallback,
+              null)
    }

    return text
@@ -482,13 +511,12 @@ internal object TextLayoutManager {

  private fun createSpannableFromAttributedString(
      context: Context,
-      attributedString: MapBuffer,
-      reactTextViewManagerCallback: ReactTextViewManagerCallback?
+      fragments: MapBuffer,
+      reactTextViewManagerCallback: ReactTextViewManagerCallback?,
+      outputReactTags: IntArray?
  ): Spannable {
    if (ReactNativeFeatureFlags.enableAndroidTextMeasurementOptimizations()) {
-      val spannable =
-          buildSpannableFromFragmentsOptimized(
-              context, attributedString.getMapBuffer(AS_KEY_FRAGMENTS))
+      val spannable = buildSpannableFromFragmentsOptimized(context, fragments, outputReactTags)

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

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

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

-    return PreparedLayout(layout, maximumNumberOfLines, verticalOffset)
+    return PreparedLayout(layout, maximumNumberOfLines, verticalOffset, reactTags)
  }

  @JvmStatic
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+package com.facebook.react.views.text.internal.span
+
+/**
+ * Maps a section of the text to the index of the AttributedString fragment originally used to
+ * create it.
+ */
+internal class ReactFragmentIndexSpan(val fragmentIndex: Int) : ReactSpan
@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+package com.facebook.react.views.text.internal.span
+
+import android.text.TextPaint
+import android.text.style.ClickableSpan
+import android.view.View
+import com.facebook.react.bridge.ReactContext
+import com.facebook.react.uimanager.UIManagerHelper
+import com.facebook.react.views.text.PreparedLayoutTextView
+import com.facebook.react.views.text.TextLayoutManager
+import com.facebook.react.views.view.ViewGroupClickEvent
+
+/**
+ * This class is used in [TextLayoutManager] to linkify and style a span of text with
+ * accessibilityRole="link". This is needed to make nested Text components accessible.
+ *
+ * For example, if your React component looks like this:
+ * ```js
+ * <Text>
+ * Some text with
+ * <Text onPress={onPress} accessible={true} accessibilityRole="link">a link</Text>
+ * in the middle.
+ * </Text>
+ * ```
+ */
+internal class ReactLinkSpan(val fragmentIndex: Int) : ClickableSpan(), ReactSpan {
+  override fun onClick(view: View) {
+    val context = view.context as ReactContext
+    val textView = view as? PreparedLayoutTextView ?: return
+    val preparedLayout = textView.preparedLayout ?: return
+    val reactTag = preparedLayout.reactTags[fragmentIndex]
+    val eventDispatcher = UIManagerHelper.getEventDispatcherForReactTag(context, reactTag)
+    eventDispatcher?.dispatchEvent(
+        ViewGroupClickEvent(UIManagerHelper.getSurfaceId(context), reactTag))
+  }
+
+  override fun updateDrawState(ds: TextPaint) {
+    // no super call so we don't change the link color or add an underline by default, as the
+    // superclass does.
+  }
+}
@@ -310,14 +310,19 @@ TextLayoutManager::PreparedLayout TextLayoutManager::prepareLayout(
              jfloat,
              jfloat)>("prepareTextLayout");

-  return preparedTextCache_.get(
+  static auto reusePreparedLayoutWithNewReactTags =
+      jni::findClassStatic("com/facebook/react/fabric/FabricUIManager")
+          ->getMethod<JPreparedLayout::javaobject(
+              JPreparedLayout::javaobject, jintArray)>(
+              "reusePreparedLayoutWithNewReactTags");
+
+  const auto [key, preparedText] = preparedTextCache_.getWithKey(
      {.attributedString = attributedString,
       .paragraphAttributes = paragraphAttributes,
       .layoutConstraints = layoutConstraints},
-      [&] {
+      [&]() {
        const auto& fabricUIManager =
            contextContainer_->at<jni::global_ref<jobject>>("FabricUIManager");
-
        auto attributedStringMB = JReadableMapBuffer::createWithContents(
            toMapBuffer(attributedString));
        auto paragraphAttributesMB = JReadableMapBuffer::createWithContents(
@@ -336,6 +341,39 @@ TextLayoutManager::PreparedLayout TextLayoutManager::prepareLayout(
            minimumSize.height,
            maximumSize.height))};
      });
+
+  // PreparedTextCacheKey allows equality of layouts which are the same
+  // display-wise, but ShadowView fragments (and thus react tags) may have
+  // changed.
+  const auto& fragments = attributedString.getFragments();
+  const auto& cacheKeyFragments = key->attributedString.getFragments();
+  bool needsNewReactTags = [&] {
+    for (size_t i = 0; i < fragments.size(); i++) {
+      if (fragments[i].parentShadowView.tag !=
+          cacheKeyFragments[i].parentShadowView.tag) {
+        return true;
+      }
+    }
+    return false;
+  }();
+
+  if (needsNewReactTags) {
+    std::vector<int> reactTags(fragments.size());
+    for (size_t i = 0; i < reactTags.size(); i++) {
+      reactTags[i] = fragments[i].parentShadowView.tag;
+    }
+
+    auto javaReactTags = jni::JArrayInt::newArray(fragments.size());
+    javaReactTags->setRegion(
+        0, static_cast<jsize>(reactTags.size()), reactTags.data());
+
+    const auto& fabricUIManager =
+        contextContainer_->at<jni::global_ref<jobject>>("FabricUIManager");
+    return PreparedLayout{jni::make_global(reusePreparedLayoutWithNewReactTags(
+        fabricUIManager, preparedText->get(), javaReactTags.get()))};
+  } else {
+    return PreparedLayout{*preparedText};
+  }
 }

 TextMeasurement TextLayoutManager::measurePreparedLayout(
@@ -38,24 +38,20 @@ class SimpleThreadSafeCache {
   */
  ValueT get(const KeyT& key, CacheGeneratorFunction<ValueT> auto generator)
      const {
-    std::lock_guard<std::mutex> lock(mutex_);
+    return getMapIterator(key, std::move(generator))->second->second;
+  }

-    if (auto it = map_.find(key); it != map_.end()) {
-      // Move accessed item to front of list
-      list_.splice(list_.begin(), list_, it->second);
-      return it->second->second;
-    }
-
-    auto value = generator();
-    // Add new value to front of list and map
-    list_.emplace_front(key, value);
-    map_[key] = list_.begin();
-    if (list_.size() > maxSize_) {
-      // Evict least recently used item (back of list)
-      map_.erase(list_.back().first);
-      list_.pop_back();
-    }
-    return value;
+  /*
+   * Returns pointers to both the key and value from the map with a given key.
+   * If the value wasn't found in the cache, constructs the value using given
+   * generator function, stores it inside a cache and returns it.
+   * Can be called from any thread.
+   */
+  std::pair<const KeyT*, const ValueT*> getWithKey(
+      const KeyT& key,
+      CacheGeneratorFunction<ValueT> auto generator) const {
+    auto it = getMapIterator(key, std::move(generator));
+    return std::make_pair(&it->first, &it->second->second);
  }

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

+  auto getMapIterator(
+      const KeyT& key,
+      CacheGeneratorFunction<ValueT> auto generator) const {
+    std::lock_guard<std::mutex> lock(mutex_);
+
+    if (auto it = map_.find(key); it != map_.end()) {
+      // Move accessed item to front of list
+      list_.splice(list_.begin(), list_, it->second);
+      return it;
+    }
+
+    auto value = generator();
+    // Add new value to front of list and map
+    list_.emplace_front(key, value);
+    auto [it, _] = map_.insert_or_assign(key, list_.begin());
+    if (list_.size() > maxSize_) {
+      // Evict least recently used item (back of list)
+      map_.erase(list_.back().first);
+      list_.pop_back();
+    }
+    return it;
+  }
+
  size_t maxSize_;
  mutable std::mutex mutex_;
  mutable std::list<EntryT> list_;