textPublisher crash when using emoji

[combinecommunity]

textPublisher crash when using emoji

[ZvonimirMedak]

With the addition on my PR #73, the example @kevinrenskers provided doesn't crash but you do get a circular dependency, and the app loops since you're saving the values from the textView.textPublisher to another publisher and then assigning the same value to the textView again.

Why the textView.textPublisher gets called after adding an emoji is that the emoji uses a different font and it will send the editingAttributes action to the didProcessEditingRangeChangeInLengthPublisher.

[kevinrenskers]

When I switch to the good ol' textViewDidChange delegate method, no crash. It also doesn't crash using UITextField, only UITextView.

[freak4pc]

Can you attach a repro project to save some time? @kevinrenskers
Thanks

[kevinrenskers]

Will do.

[kevinrenskers]

CombineCocoaTest.zip

[freak4pc]

Hey @kevinrenskers - I don't think there's any bug with the library per-se here, since if you replace it with a print, everything works:

I think the real problem is that you're introducing reentrancy - you are assigning a value of something that effects that very same value. In this specific case the crash is:

Attempted layout while textStorage is editing. It is not valid to cause the layoutManager to do layout while the textStorage is editing (ie the textStorage has been sent a beginEditing message without a matching endEditing.)

Which seems reasonable (you are editing and writing, e.g. causing a layout at the exact same time or at a timing that causes a reentrancy/race).

[kevinrenskers]

But it works with all text just fine, just not emoji.

[freak4pc]

Yup, I imagine drawing grapheme clusters is a different task that requires different handling. Regardless this error comes from NSTextStorage and not from CombineCocoa itself. I'm not too sure if I have an idea for something that could be done here.

[kevinrenskers]

I wonder if it's something in didProcessEditingRangeChangeInLengthPublisher.

Like I said, if you build the exact same functionality using good old textViewDidChange, it doesn't crash.

import UIKit
import Combine
import CombineCocoa

class SearchViewController: UIViewController {
  @IBOutlet var textView1: UITextView!
  @IBOutlet var textView2: UITextView!

  @Published var value: String? = ""

  private var subscriptions = Set<AnyCancellable>()

  override func viewDidLoad() {
    super.viewDidLoad()

    $value
      .sink {
        self.textView1.text = $0
        self.textView2.text = $0
      }
      .store(in: &subscriptions)
  }
}

extension SearchViewController: UITextViewDelegate {
  func textViewDidChange(_ textView: UITextView) {
    value = textView.text
  }
}

[freak4pc]

There is nothing reentrant about this code though. I don't think the delegate gets called when the sink hits the .text setter, but it does happen with assign for some reason.

[kevinrenskers]

So, it's not the assigning that's the problem (I still have the $value.sink code), but reading the value that seems to trip it up - the textPublisher.

[kevinrenskers]

It also doesn't crash if you replace both UITextView with a UITextField, which don't use the didProcessEditingRangeChangeInLengthPublisher, which is why my mind is going in that direction.

[freak4pc]

UITextView is a beast of a controller. It is nothing like UITextField which isn’t backed by NSTextStorage. Ill try to fiddle with it to see if I have some ideas on how to avoid or defer the reentrancy somehow. Thanks!

…

On 21 Nov 2020, 21:44 +0200, Kevin Renskers ***@***.***>, wrote: It also doesn't crash if you replace both UITextView with a UITextField, which don't use the didProcessEditingRangeChangeInLengthPublisher, which is why my mind is going in that direction. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.