Fixes for AttributedString Comparison (#50147)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/50147

Noticed a couple bugs here, around a crash from assertion internal to the caching map which hashes on the AttributedString, that may or may not be related.

1. We are missing `baseTextAttributes` for both hashing and equality (which is mostly innocuous, but still wrong)
2. We were not hashing or comparing `textAlignVertical`
3. For equality, we were comparing parent shadow view tag and metrics, but for hashing, we were hashing the whole ShadowView.

I think #3 could cause issues, since we could see different hash despite equality, which could break invariants.

Changelog: [Internal]

Reviewed By: lunaleaps

Differential Revision: D71500246

fbshipit-source-id: 462749d5ca10d10bf0dab88089253a2bb8e603fb
This commit is contained in:
Nick Gerleman
2025-03-19 19:42:30 -07:00
committed by Facebook GitHub Bot
parent 7488f66fe5
commit 7136be22fa
4 changed files with 11 additions and 20 deletions
@@ -47,10 +47,6 @@ bool Fragment::isContentEqual(const Fragment& rhs) const {
std::tie(rhs.string, rhs.textAttributes);
}
bool Fragment::operator!=(const Fragment& rhs) const {
return !(*this == rhs);
}
#pragma mark - AttributedString
void AttributedString::appendFragment(Fragment&& fragment) {
@@ -113,11 +109,8 @@ bool AttributedString::compareTextAttributesWithoutFrame(
}
bool AttributedString::operator==(const AttributedString& rhs) const {
return fragments_ == rhs.fragments_;
}
bool AttributedString::operator!=(const AttributedString& rhs) const {
return !(*this == rhs);
return std::tie(fragments_, baseAttributes_) ==
std::tie(rhs.fragments_, rhs.baseAttributes_);
}
bool AttributedString::isContentEqual(const AttributedString& rhs) const {
@@ -51,7 +51,6 @@ class AttributedString : public Sealable, public DebugStringConvertible {
bool isContentEqual(const Fragment& rhs) const;
bool operator==(const Fragment& rhs) const;
bool operator!=(const Fragment& rhs) const;
};
class Range {
@@ -104,7 +103,6 @@ class AttributedString : public Sealable, public DebugStringConvertible {
bool isContentEqual(const AttributedString& rhs) const;
bool operator==(const AttributedString& rhs) const;
bool operator!=(const AttributedString& rhs) const;
#pragma mark - DebugStringConvertible
@@ -127,7 +125,7 @@ struct hash<facebook::react::AttributedString::Fragment> {
return facebook::react::hash_combine(
fragment.string,
fragment.textAttributes,
fragment.parentShadowView,
fragment.parentShadowView.tag,
fragment.parentShadowView.layoutMetrics);
}
};
@@ -138,6 +136,8 @@ struct hash<facebook::react::AttributedString> {
const facebook::react::AttributedString& attributedString) const {
auto seed = size_t{0};
facebook::react::hash_combine(
seed, attributedString.getBaseTextAttributes());
for (const auto& fragment : attributedString.getFragments()) {
facebook::react::hash_combine(seed, fragment);
}
@@ -146,7 +146,8 @@ bool TextAttributes::operator==(const TextAttributes& rhs) const {
layoutDirection,
accessibilityRole,
role,
textTransform) ==
textTransform,
textAlignVertical) ==
std::tie(
rhs.foregroundColor,
rhs.backgroundColor,
@@ -170,7 +171,8 @@ bool TextAttributes::operator==(const TextAttributes& rhs) const {
rhs.layoutDirection,
rhs.accessibilityRole,
rhs.role,
rhs.textTransform) &&
rhs.textTransform,
rhs.textAlignVertical) &&
floatEquality(maxFontSizeMultiplier, rhs.maxFontSizeMultiplier) &&
floatEquality(opacity, rhs.opacity) &&
floatEquality(fontSize, rhs.fontSize) &&
@@ -180,10 +182,6 @@ bool TextAttributes::operator==(const TextAttributes& rhs) const {
floatEquality(textShadowRadius, rhs.textShadowRadius);
}
bool TextAttributes::operator!=(const TextAttributes& rhs) const {
return !(*this == rhs);
}
TextAttributes TextAttributes::defaultTextAttributes() {
static auto textAttributes = [] {
auto textAttributes = TextAttributes{};
@@ -95,7 +95,6 @@ class TextAttributes : public DebugStringConvertible {
#pragma mark - Operators
bool operator==(const TextAttributes& rhs) const;
bool operator!=(const TextAttributes& rhs) const;
#pragma mark - DebugStringConvertible
@@ -142,7 +141,8 @@ struct hash<facebook::react::TextAttributes> {
textAttributes.isPressable,
textAttributes.layoutDirection,
textAttributes.accessibilityRole,
textAttributes.role);
textAttributes.role,
textAttributes.textAlignVertical);
}
};
} // namespace std