Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(textinput): backporting of fixes for next 0.71 patch release #36874

Merged
merged 10 commits into from
Apr 17, 2023

Commits on Apr 11, 2023

  1. Fix measurement of uncontrolled TextInput after edit

    Summary:
    D42721684 (be69c8b) left a pretty bad bug when using Fabric for Android. I missed that in Fabric specifically, on edit we will cache the Spannable backing the EditText for use in future measurement.
    
    Because we've stripped the sizing spans, Spannable measurement has incorrect font size, and the TextInput size will change (collapsing) after the first edit. This effectively breaks any uncontrolled TextInput which does not have explicit dimensions set.
    
    Changelog:
    [Android][Fixed] - Fix measurement of uncontrolled TextInput after edit
    
    Reviewed By: sammy-SC
    
    Differential Revision: D43158407
    
    fbshipit-source-id: 51602eab06c9a50e2b60ef0ed87bdb4df025e51e
    NickGerleman authored and kelset committed Apr 11, 2023
    Configuration menu
    Copy the full SHA
    0d3600b View commit details
    Browse the repository at this point in the history
  2. Minimize EditText Spans 1/9: Fix precedence (#36543)

    Summary:
    Pull Request resolved: #36543
    
    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
    
    Reviewed By: javache
    
    Differential Revision: D44240779
    
    fbshipit-source-id: f731b353587888faad946b8cf1e868095cdeced3
    NickGerleman authored and kelset committed Apr 11, 2023
    Configuration menu
    Copy the full SHA
    8ad6496 View commit details
    Browse the repository at this point in the history
  3. Minimize EditText Spans 2/9: Make stripAttributeEquivalentSpans gener…

    …ic (#36546)
    
    Summary:
    Pull Request resolved: #36546
    
    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.
    
    This change generalizes `stripAttributeEquivalentSpans()` to allow plugging in different spans.
    
    Changelog:
    [Internal]
    
    Reviewed By: rshest
    
    Differential Revision: D44240781
    
    fbshipit-source-id: 89005266020f216368e9ad9ce382699bd8db85a8
    
    # Conflicts:
    #	ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java
    NickGerleman authored and kelset committed Apr 11, 2023
    Configuration menu
    Copy the full SHA
    e7122df View commit details
    Browse the repository at this point in the history
  4. Minimize EditText Spans 3/9: ReactBackgroundColorSpan (#36547)

    Summary:
    Pull Request resolved: #36547
    
    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.
    
    This adds `ReactBackgroundColorSpan` to the list of spans eligible to be stripped.
    
    Changelog:
    [Android][Fixed] - Minimize Spans 3/N: ReactBackgroundColorSpan
    
    Reviewed By: javache
    
    Differential Revision: D44240782
    
    fbshipit-source-id: 2ded1a1687a41cf6d5f83e89ffadd2d932089969
    NickGerleman authored and kelset committed Apr 11, 2023
    Configuration menu
    Copy the full SHA
    d145957 View commit details
    Browse the repository at this point in the history
  5. Minimize EditText Spans 4/9: ReactForegroundColorSpan (#36545)

    Summary:
    Pull Request resolved: #36545
    
    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.
    
    This adds ReactForegroundColorSpan to the list of spans eligible to be stripped.
    
    Changelog:
    [Android][Fixed] - Minimize Spans 4/N: ReactForegroundColorSpan
    
    Reviewed By: javache
    
    Differential Revision: D44240780
    
    fbshipit-source-id: d86939cc2d7ed9116a4167026c7d48928fc51757
    NickGerleman authored and kelset committed Apr 11, 2023
    Configuration menu
    Copy the full SHA
    644a3da View commit details
    Browse the repository at this point in the history
  6. Minimize EditText Spans 5/9: Strikethrough and Underline (#36544)

    Summary:
    Pull Request resolved: #36544
    
    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.
    
    This change makes us apply strikethrough and underline as paint flags to the underlying EditText, instead of just the spans. We then opt ReactUnderlineSpan and ReactStrikethroughSpan into being strippable.
    
    This does actually create visual behavior changes, where child text will inherit any underline or strikethrough of the root EditText (including if the child specifies `textDecorationLine: "none"`. The new behavior is consistent with both iOS and web though, so it seems like more of a bugfix than a regression.
    
    Changelog:
    [Android][Fixed] - Minimize Spans 5/N: Strikethrough and Underline
    
    Reviewed By: rshest
    
    Differential Revision: D44240778
    
    fbshipit-source-id: d564dfc0121057a5e3b09bb71b8f5662e28be17e
    NickGerleman authored and kelset committed Apr 11, 2023
    Configuration menu
    Copy the full SHA
    ba4c2f5 View commit details
    Browse the repository at this point in the history
  7. Minimize EditText Spans 6/9: letterSpacing (#36548)

    Summary:
    Pull Request resolved: #36548
    
    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.
    
    This change lets us set `letterSpacing` on the EditText instead of using our custom span.
    
    Changelog:
    [Android][Fixed] - Minimize EditText Spans 6/N: letterSpacing
    
    Reviewed By: rshest
    
    Differential Revision: D44240777
    
    fbshipit-source-id: 9bd10c3261257037d8cacf37971011aaa94d1a77
    NickGerleman authored and kelset committed Apr 11, 2023
    Configuration menu
    Copy the full SHA
    e89d23c View commit details
    Browse the repository at this point in the history
  8. Minimize EditText Spans 7/9: Avoid temp list (#36576)

    Summary:
    Pull Request resolved: #36576
    
    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.
    
    This change addresses some minor CR feedback and removes the temporary list of spans in favor of applying them directly.
    
    Changelog:
    [Internal]
    
    Reviewed By: javache
    
    Differential Revision: D44295190
    
    fbshipit-source-id: bd784e2c514301d45d0bacd8ee6de5c512fc565c
    NickGerleman authored and kelset committed Apr 11, 2023
    Configuration menu
    Copy the full SHA
    2e2ce8a View commit details
    Browse the repository at this point in the history
  9. Minimize EditText Spans 8/9: CustomStyleSpan (#36577)

    Summary:
    Pull Request resolved: #36577
    
    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.
    
    This change allows us to strip CustomStyleSpan. We already set all but `fontVariant` on the underlying EditText, so we just need to route that through as well.
    
    Note that because this span is non-parcelable, it is seemingly not subject to the buggy behavior on Samsung devices of infinitely cloning the spans, but non-parcelable spans have different issues on the devices (they disappear), so moving `fontVariant` to the top-level makes sense here.
    
    Changelog:
    [Android][Fixed] - Minimize EditText Spans 8/N: CustomStyleSpan
    
    Reviewed By: javache
    
    Differential Revision: D44297384
    
    fbshipit-source-id: ed4c000e961dd456a2a8f4397e27c23a87defb6e
    NickGerleman authored and kelset committed Apr 11, 2023
    Configuration menu
    Copy the full SHA
    77bd902 View commit details
    Browse the repository at this point in the history
  10. Mimimize EditText Spans 9/9: Remove addSpansForMeasurement() (#36575)

    Summary:
    Pull Request resolved: #36575
    
    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.
    
    D23670779 addedd a previous mechanism to add spans for measurement caching, like we needed to do as part of this change. It is called in more specific cases (e.g. when there is a text hint but no text), but it edits the live EditText spannable instead of the cache copy, and does not handle nested text at all.
    
    We are already adding spans back to the input after this, behind everything else, and can replace it with the code we have been adding.
    
    Changelog:
    [Android][Fixed] - Mimimize EditText Spans 9/9: Remove `addSpansForMeasurement()`
    
    Reviewed By: javache
    
    Differential Revision: D44298159
    
    fbshipit-source-id: 1af44a39de7550b7e66e45db9ebc3523ae9ff002
    
    # Conflicts:
    #	ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
    NickGerleman authored and kelset committed Apr 11, 2023
    Configuration menu
    Copy the full SHA
    181bd38 View commit details
    Browse the repository at this point in the history