From 9fe20d4ea33532abb344383873b320cdb779f5c4 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Mon, 19 May 2025 18:15:39 -0700 Subject: [PATCH] Remove Measurement Placeholder Logic from Text ShadowNodes (#51465) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/51465 Prior to D63303709 AttributedString could not represent formatting on an empty string, and so some text content was forcefully added to empty strings during measurement. This is problematic in combination with Facsimile, where we directly render the layout we used during measurement, since empty text now has a random "I" in it. Android's TextLayoutManager already knows how to interpret `baseTextAttributes`, and the placeholder is not needed. Other platforms should be updated to do the same, but that may be non-trivial to validate everywhere. This diff removes logic from ShadowNodes to always inset a placeholder, and instead shims it in platform TextLayoutManagers which have not yet been updated to use `BaseTextAttributes`. That way, we don't force placeholders during measurement, and different platforms can incrementally unjank their code. Changelog: [Internal] Reviewed By: rshest Differential Revision: D74770916 fbshipit-source-id: 7cf19db1a9a5cf68137bbff81b14ce5288235b2b --- .../PlaceholderAttributedString.h | 34 +++++++++++++++++++ .../components/text/BaseTextShadowNode.h | 7 ---- .../components/text/ParagraphShadowNode.cpp | 32 ++--------------- .../textinput/BaseTextInputShadowNode.h | 24 +++++-------- .../AndroidTextInputShadowNode.cpp | 29 +++++----------- .../textlayoutmanager/TextLayoutManager.mm | 5 +-- 6 files changed, 58 insertions(+), 73 deletions(-) create mode 100644 packages/react-native/ReactCommon/react/renderer/attributedstring/PlaceholderAttributedString.h diff --git a/packages/react-native/ReactCommon/react/renderer/attributedstring/PlaceholderAttributedString.h b/packages/react-native/ReactCommon/react/renderer/attributedstring/PlaceholderAttributedString.h new file mode 100644 index 00000000000..3b8329eb653 --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/attributedstring/PlaceholderAttributedString.h @@ -0,0 +1,34 @@ +/* + * 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. + */ + +#pragma once + +#include + +namespace facebook::react { + +/** + * Prior to D63303709 AttributedString could not represent formatting on an + * empty string, and so some text content was forcefully added to empty strings + * during measurement. Usages of this function should be replaced with + * formatting based off of baseTextAttributes. + */ +inline AttributedString ensurePlaceholderIfEmpty_DO_NOT_USE( + const AttributedString& attributedString) { + if (attributedString.isEmpty()) { + AttributedString placeholder{attributedString}; + placeholder.appendFragment( + {.string = "I", + .textAttributes = attributedString.getBaseTextAttributes(), + .parentShadowView = {}}); + return placeholder; + } + + return attributedString; +} + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/components/text/BaseTextShadowNode.h b/packages/react-native/ReactCommon/react/renderer/components/text/BaseTextShadowNode.h index ec281eb7e2f..5d50e92a2c9 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/text/BaseTextShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/components/text/BaseTextShadowNode.h @@ -56,13 +56,6 @@ class BaseTextShadowNode { const ShadowNode& parentNode, AttributedString& outAttributedString, Attachments& outAttachments); - - /** - * Returns a character used to measure empty strings in native platforms. - */ - inline static std::string getEmptyPlaceholder() { - return "I"; - } }; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp index b4249da43cc..6a323fed40a 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp @@ -244,19 +244,6 @@ Size ParagraphShadowNode::measureContent( auto content = getContentWithMeasuredAttachments(layoutContext, layoutConstraints); - auto attributedString = content.attributedString; - if (attributedString.isEmpty()) { - // Note: `zero-width space` is insufficient in some cases (e.g. when we - // need to measure the "height" of the font). - // TODO T67606511: We will redefine the measurement of empty strings as - // part of T67606511 - auto string = BaseTextShadowNode::getEmptyPlaceholder(); - auto textAttributes = TextAttributes::defaultTextAttributes(); - textAttributes.fontSizeMultiplier = layoutContext.fontSizeMultiplier; - textAttributes.apply(getConcreteProps().textAttributes); - attributedString.appendFragment({string, textAttributes, {}}); - } - TextLayoutContext textLayoutContext{ .pointScaleFactor = layoutContext.pointScaleFactor, .surfaceId = getSurfaceId(), @@ -267,7 +254,7 @@ Size ParagraphShadowNode::measureContent( TextLayoutManagerExtended tme(*textLayoutManager_); auto preparedLayout = tme.prepareLayout( - attributedString, + content.attributedString, content.paragraphAttributes, textLayoutContext, layoutConstraints); @@ -287,7 +274,7 @@ Size ParagraphShadowNode::measureContent( auto size = textLayoutManager_ ->measure( - AttributedStringBox{attributedString}, + AttributedStringBox{content.attributedString}, content.paragraphAttributes, textLayoutContext, layoutConstraints) @@ -304,21 +291,8 @@ Float ParagraphShadowNode::baseline( LayoutConstraints{size, size, layoutMetrics.layoutDirection}; auto content = getContentWithMeasuredAttachments(layoutContext, layoutConstraints); - auto attributedString = content.attributedString; - if (attributedString.isEmpty()) { - // Note: `zero-width space` is insufficient in some cases (e.g. when we - // need to measure the "height" of the font). - // TODO T67606511: We will redefine the measurement of empty strings as - // part of T67606511 - auto string = BaseTextShadowNode::getEmptyPlaceholder(); - auto textAttributes = TextAttributes::defaultTextAttributes(); - textAttributes.fontSizeMultiplier = layoutContext.fontSizeMultiplier; - textAttributes.apply(getConcreteProps().textAttributes); - attributedString.appendFragment({string, textAttributes, {}}); - } - - AttributedStringBox attributedStringBox{attributedString}; + AttributedStringBox attributedStringBox{content.attributedString}; if constexpr (TextLayoutManagerExtended::supportsLineMeasurement()) { auto lines = diff --git a/packages/react-native/ReactCommon/react/renderer/components/textinput/BaseTextInputShadowNode.h b/packages/react-native/ReactCommon/react/renderer/components/textinput/BaseTextInputShadowNode.h index 157666a8159..8c8a225eaa9 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/textinput/BaseTextInputShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/components/textinput/BaseTextInputShadowNode.h @@ -228,26 +228,20 @@ class BaseTextInputShadowNode : public ConcreteViewShadowNode< return AttributedStringBox{attributedString}; } - // For measurement purposes, we want to make sure that there's at least a - // single character in the string so that the measured height is greater - // than zero. Otherwise, empty TextInputs with no placeholder don't - // display at all. - // TODO T67606511: We will redefine the measurement of empty strings as part - // of T67606511 AttributedString getPlaceholderAttributedString( const LayoutContext& layoutContext) const { const auto& props = BaseShadowNode::getConcreteProps(); AttributedString attributedString; - auto placeholderString = !props.placeholder.empty() - ? props.placeholder - : BaseTextShadowNode::getEmptyPlaceholder(); - auto textAttributes = - props.getEffectiveTextAttributes(layoutContext.fontSizeMultiplier); - attributedString.appendFragment( - {.string = std::move(placeholderString), - .textAttributes = textAttributes, - .parentShadowView = {}}); + attributedString.setBaseTextAttributes( + props.getEffectiveTextAttributes(layoutContext.fontSizeMultiplier)); + + if (!props.placeholder.empty()) { + attributedString.appendFragment( + {.string = props.placeholder, + .textAttributes = attributedString.getBaseTextAttributes(), + .parentShadowView = {}}); + } return attributedString; } }; diff --git a/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp index 524e5184f74..e9a546e0154 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp @@ -60,10 +60,6 @@ Size AndroidTextInputShadowNode::measureContent( attributedString = getPlaceholderAttributedString(layoutContext); } - if (attributedString.isEmpty() && getStateData().mostRecentEventCount != 0) { - return {.width = 0, .height = 0}; - } - auto textSize = textLayoutManager_ ->measure( AttributedStringBox{attributedString}, @@ -217,27 +213,20 @@ AttributedString AndroidTextInputShadowNode::getMostRecentAttributedString( : reactTreeAttributedString); } -// For measurement purposes, we want to make sure that there's at least a -// single character in the string so that the measured height is greater -// than zero. Otherwise, empty TextInputs with no placeholder don't -// display at all. -// TODO T67606511: We will redefine the measurement of empty strings as part -// of T67606511 AttributedString AndroidTextInputShadowNode::getPlaceholderAttributedString( const LayoutContext& layoutContext) const { const auto& props = BaseShadowNode::getConcreteProps(); AttributedString attributedString; - auto placeholderString = !props.placeholder.empty() - ? props.placeholder - : BaseTextShadowNode::getEmptyPlaceholder(); - auto textAttributes = TextAttributes::defaultTextAttributes(); - textAttributes.fontSizeMultiplier = layoutContext.fontSizeMultiplier; - textAttributes.apply(props.textAttributes); - attributedString.appendFragment( - {.string = std::move(placeholderString), - .textAttributes = textAttributes, - .parentShadowView = ShadowView(*this)}); + attributedString.setBaseTextAttributes( + props.getEffectiveTextAttributes(layoutContext.fontSizeMultiplier)); + + if (!props.placeholder.empty()) { + attributedString.appendFragment( + {.string = props.placeholder, + .textAttributes = attributedString.getBaseTextAttributes(), + .parentShadowView = {}}); + } return attributedString; } diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm index 3220adcb908..f4f7ba80b66 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm @@ -8,6 +8,7 @@ #import "TextLayoutManager.h" #import "RCTTextLayoutManager.h" +#import #import #import @@ -36,7 +37,7 @@ TextMeasurement TextLayoutManager::measure( switch (attributedStringBox.getMode()) { case AttributedStringBox::Mode::Value: { - auto &attributedString = attributedStringBox.getValue(); + auto attributedString = ensurePlaceholderIfEmpty_DO_NOT_USE(attributedStringBox.getValue()); measurement = textMeasureCache_.get( {attributedString, paragraphAttributes, layoutConstraints}, [&](const TextMeasureCacheKey &key) { @@ -92,7 +93,7 @@ LinesMeasurements TextLayoutManager::measureLines( const Size &size) const { react_native_assert(attributedStringBox.getMode() == AttributedStringBox::Mode::Value); - const auto &attributedString = attributedStringBox.getValue(); + auto attributedString = ensurePlaceholderIfEmpty_DO_NOT_USE(attributedStringBox.getValue()); RCTTextLayoutManager *textLayoutManager = (RCTTextLayoutManager *)unwrapManagedObject(nativeTextLayoutManager_);