Skip to content

Commit

Permalink
Minimize EditText Spans 1/N: Fix precedence
Browse files Browse the repository at this point in the history
Summary:
This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( #35936 (comment)) for greater context on the platform behavior.

We cache the backing EditText span on text change to later measure. To measure outside of a TextInput we need to restore any spans we removed. Spans may overlap, so base attributes should be behind everything else.

The logic here for dealing with precedence is incorrect, and we should instead accomplish this by twiddling with the `SPAN_PRIORITY` bits.

Changelog:
[Android][Fixed] - Minimize Spans 1/N: Fix precedence

Differential Revision: D44240779

fbshipit-source-id: 1a9c1fb2b6f7d7d95c14752f73ea2b6f3bdad2c3
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Mar 21, 2023
1 parent 6956d4f commit 637f494
Showing 1 changed file with 8 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -683,29 +683,18 @@ private void stripAttributeEquivalentSpans(SpannableStringBuilder sb) {
}
}

private void unstripAttributeEquivalentSpans(
SpannableStringBuilder workingText, Spannable originalText) {
// We must add spans back for Fabric to be able to measure, at lower precedence than any
// existing spans. Remove all spans, add the attributes, then re-add the spans over
workingText.append(originalText);
private void unstripAttributeEquivalentSpans(SpannableStringBuilder workingText) {
int spanFlags = Spannable.SPAN_INCLUSIVE_INCLUSIVE;

for (Object span : workingText.getSpans(0, workingText.length(), Object.class)) {
workingText.removeSpan(span);
}
// Set all bits for SPAN_PRIORITY so that this span has the highest possible priority
// (least precedence). This ensures the span is behind any overlapping spans.
spanFlags |= Spannable.SPAN_PRIORITY;

workingText.setSpan(
new ReactAbsoluteSizeSpan(mTextAttributes.getEffectiveFontSize()),
0,
workingText.length(),
Spanned.SPAN_INCLUSIVE_INCLUSIVE);

for (Object span : originalText.getSpans(0, originalText.length(), Object.class)) {
workingText.setSpan(
span,
originalText.getSpanStart(span),
originalText.getSpanEnd(span),
originalText.getSpanFlags(span));
}
spanFlags);
}

private static boolean sameTextForSpan(
Expand Down Expand Up @@ -1132,8 +1121,8 @@ private void updateCachedSpannable(boolean resetStyles) {
// ...
// - android.app.Activity.dispatchKeyEvent (Activity.java:3447)
try {
Spannable text = (Spannable) currentText.subSequence(0, currentText.length());
unstripAttributeEquivalentSpans(sb, text);
sb.append(currentText.subSequence(0, currentText.length()));
unstripAttributeEquivalentSpans(sb);
} catch (IndexOutOfBoundsException e) {
ReactSoftExceptionLogger.logSoftException(TAG, e);
}
Expand Down

0 comments on commit 637f494

Please sign in to comment.