Japanese text would be broken if UITextField has two-way-binding ControlProperty and Variable.

[tarunon]

I use two-way-binding operator UITextField.rx_text and Variable like RxExample.
https://github.com/ReactiveX/RxSwift/blob/master/RxExample/RxExample/Examples/APIWrappers/APIWrappersViewController.swift#L142-L149

And please see this capture.

I want to input "あいうえお" at the textField, but we can see a wrong text "ああいあいうあいうえあいうえお".
I believe ControlProperty should not receive event about the value inputted from user, because textField would lost the invisible state when set the same text.

And UITextView has the nearly same problem.
In UITextView, we cannot use IME character conversion.

This problem would occur in every language that has IME.

I believe the easy way to solve this problem is change the implements of two-way-binding operator.
But... I think ControlProperty(or rx_value) might check the observed value is not equal to a value inputted from user.

[omochi]

I am a japanese developer also faced same problem.
I have not looked into what is happening in deeply.
But I found that I can avoid the problem by blocking event
which is comming from ControlProperty itself via Variable.
So I made this extension and be using it now.
(But it may occurs another problem in some specific cases.)
Of course I am happy if it is fixed in RxSwift.
This is just information for others with this problem.

extension ControlProperty {
    func cyclePrevented() -> ControlProperty<PropertyType> {
        var fromControl = false
        let observable = Observable<PropertyType>.create { observer in
            let sub = self.subscribe { ev in
                fromControl = true

                observer.on(ev)

                fromControl = false
            }
            return sub
        }
        let observer = AnyObserver<PropertyType> { ev in
            if !fromControl {
                self.on(ev)
            }
        }
        return ControlProperty(values: observable, valueSink: observer)
    }
}

user.name <-> userNameField.rx_text.cyclePrevented()

[sergdort]

Hi, guys

Looks like this is related to #626

I removed distinctUntilChanged in the UIControl.rx_value and it fixed the problem for me

[kzaher]

Thnx for investigation 👍 @sergdort

@tarunon @omochi I've pushed all changes we had to master branch. It would be great if you could verify that current master branch fixes your issue before we create another release.

[tarunon]

Thank you @omochi @sergdort @kzaher

But this problem would occur in latest master branch still...
I believe this problem is caused in ControlProperty.
In ControlProperty, If it have two-way-binding with Variable, _valueSink receive event that is observed from own _value.

UITextField has a state outside of UITextField.text while we input Japanese.
I name that is undetermined text.

For example, I explain the case of I input "あ" in UITextField.
UITextField.text is "あ", but it is the undertermined text.
And input next character "い", UITextField would replace undetermined text "あ" to new text "あい".
And UITextField lose undetermined text when set new text programatically.

In this case, we input "あ" first, then ControlProperty set new text, and UITextField lose undetermined text "あ".
We input "い" second, UITextField lost the undetermined text "あ", but new text is "あい".
Finally, UITextField would set new text "あ" + "あい".

I think @omochi 's extension is work well, but there is no check the event is realy from ownself.

[omochi]

I try to explain more detail of @tarunon said.
UITextField have internal two variable which are determined text and undetermined text.
This treats .text property as their concatenation.
If we update .text property, it updates determined text and clear undetermined text.
The keyboard remembers previous text user inputted and recover this from memory when user input next character.
This behavior is need to conversion such like "あい"(hiragana) to "愛"(kanji).
For evidence, "愛" is shown in conversion candidates on top of keyboard in his GIF movie.

So his scenario is able to written as below.

First state.
(.text: "", det: "", undet: "", keyboard: "")
When user inputs "あ", keyboard set undet text to textview.
(.text: "あ", det: "", undet: "あ", keyboard: "あ")
.text is "あ", so ControlProperty emits "あ"
A emitted "あ" loopbacks and set to .text.
In this point, the keyboard reminders previous "あ" but does not apply it to textview until user changes keyboard state.
(.text: "あ", .det: "あ", undet: "", keyboard: "あ")
Then user inputs "い", keyboard updates textviews undet text.
(.text: "ああい", .det: "あ", undet: "あい", keyboard: "あい")

@tarunon Is my understanding same to you?

[Succete]

I try it too. And I found an interesting result.

first case:

textField.rx_text
            .debug("text")
            .subscribeNext { text in
                print(text)
            }
            .addDisposableTo(disposeBag)

It's true.

second case

let textValue = Variable("")
        textField.rx_text <-> textValue

        textValue.asObservable()
            .subscribeNext { text in
                print(text)
            }
            .addDisposableTo(disposeBag)

        textField.rx_text
            .subscribeNext { text in
                print(text)
            }
            .addDisposableTo(disposeBag)

they are both false.

and there are snapshots
first case

second case

It seems that in first case keyboard is waiting my confirm, but second case not.
I think maybe the <-> operator is the reason for the bug.
let bindToUIDisposable = variable.asObservable() .bindTo(property)
this code bind variable's value to property. When I type "あ", the value has been set to property(but it should wait for my confirm before it is set).

[tarunon]

@omochi
Thanks for your explain, yes it is just my thinking.

@Succete
Yes, I believe we can fix two-way-binding operator, and It is easy way to solve this problem.
On the other hand, I worry about specification of ControlProperty.
"ControlProperty should (or not) receive event that is notified ownself."

[omochi]

@Succete Thank you for a suggestive report.
I was able to reproduce it, and tap to "愛" in conversion candidates.
Then fields presents "ああい愛".
Keyboard may replace undetermined text in UITextView to its determined conversion result in ordinary cases.
If we set UITextView.text, undetermined text is cleared and selecting with blue shadow is reset.
So conversion result is just appended without replacing.

[Succete]

@tarunon @omochi Thank you guys for replying.

[Succete]

I think I know how to solve it. It's not the two way binding that need to be changed. this is the problem

public var rx_nomarkedtext: ControlProperty<String> {
        return UIControl.rx_value(
            self,
            getter: { textField in
                textField.text ?? ""
            }, setter: { textField, value in
                if textField.markedTextRange == nil {
                    textField.text = value
                }
            }
        )
    }

then it's correct

let textValue = Variable("")
        textField.rx_nomarkedtext <-> textValue

        textValue.asObservable()
            .debug("value")
            .subscribeNext { text in
                print(text)
            }
            .addDisposableTo(disposeBag)

        textField.rx_text
            .debug("rx_text")
            .subscribeNext { text in
                print(text)
            }
            .addDisposableTo(disposeBag)

[kzaher]

Hi guys,

That's an interesting discussion for sure :)

It seems to me that adding something like

/**
    Reactive wrapper for `text` property. Notifications are fired only when when there
    isn't any marked text.
    */
    public var rx_nonMarkedText: ControlProperty<String> {
        let textProperty = self.rx_text
        return ControlProperty(
            values: textProperty._values.filter { [weak self] _ in self?.markedTextRange == nil },
            valueSink: textProperty._valueSink
        )
    }

         let textValue = Variable("")
        textField.rx_nonMarkedText <-> textValue

        textValue.asObservable()
            .subscribeNext { [weak self] x in
                self?.debug("UITextField text \(x)")
            }
            .addDisposableTo(disposeBag)

would solve your problems. I don't understand the full culture/language context so if you could provide feedback, that would be great.

[Succete]

Hi @kzaher , it's great.
I've tried all Chinese and Japanese Keyboards, and it works fine. I think we can add it to UITextView too.

[tarunon]

Hi @kzaher
Thank you for your great solution. It's seems solve this problem.
I found a problem in this solution.
In the case of save text, this solution is perfect.
But in the case of query search using suggest list, the user might expect that they can get suggest list from undetermined text.

I think the best of solve this problem is using properly rx_text and rx_nonMarkedText.

[kzaher]

Hi @tarunon @Succete @omochi ,

yeah I was thinking of having two properties. Both for UITextField and UITextView.

The other one being

/**
    Reactive wrapper for `text` property. Notifications are fired only when when there
    isn't any marked text.
    */
    public var rx_nonMarkedText: ControlProperty<String> {
        let textProperty = self.rx_text
        return ControlProperty(
            values: textProperty._values.filter { [weak self] _ in self?.markedTextRange == nil },
            valueSink: textProperty._valueSink
        )
    }

so if we did that, would that make sense and solve all issues?

[Succete]

Hi @kzaher ,
I think that would solve all issues.

[tarunon]

Hi @kzaher ,
I think we can solve all issues.
If possible, I recommend using properly rx_text(rx_nonMarkedText) and rx_markedText for convenience and safety.

/**
 Reactive wrapper for `text` property.
*/
private var rx_textInternal: ControlProperty<String> {
    return UIControl.rx_value(
        self,
        getter: { textField in
            textField.text ?? ""
        }, setter: { textField, value in
            textField.text = value
        }
    )
}

/**
 Reactive wrapper for `text` property. Notifications are fired only when when there
 isn't any marked text.
 */
public var rx_text: ControlProperty<String> {
    let textProperty = self.rx_textInternal
    return ControlProperty(
        values: textProperty._values.filter { [weak self] _ in self?.markedTextRange == nil },
        valueSink: textProperty._valueSink
    )
}

/**
 Reactive wrapper for `text` property. Notifications are fired only when when there
 is marked text.
 */
public var rx_markedText: ControlProperty<String> {
    let textProperty = self.rx_textInternal
    return ControlProperty(
        values: textProperty._values.filter { [weak self] _ in self?.markedTextRange != nil },
        valueSink: textProperty._valueSink
    )
}

I think we satisfy using rx_nonMarkedText in many cases, and we need rx_markedText in the limited cases.

[omochi]

@tarunon
When we use one-way binding, there is no problem.
The problem is in two-way binding only.
And current rx_text behavior is useful for searching text field.
But your proposal bring change of behavior in such existing code.
And I don't want to merge your rx_text and your rx_markedText to recover original function.
Its confusing.

@kzaher
Your proposal can solve all problems if programmers know rx_nonMarkedText.
I think that some Japanese RxSwift beginners (recently increasing) will step on trap of rx_text. (like me)
It is better that default option (rx_text) is safer than specific option (rx_nonMarkedText).
But as noted above, changing rx_text brings another problem.
So I propose to add information about rx_nonMarkedText in summary source comment on rx_text.

[tarunon]

@omochi
Yes, I can understand you say.
But, UITextView has same problem, and now, we must not use two-way binding at rx_text and any Variable. I think it is bug, and we can fix it.
I recommended using rx_markedText a few days ago.
But now I think rx_markedText type might be Observable. (Because we would not use two-way binding).

Sorry I misunderstood PublishSubject. It is not appropriate in this case.

[kzaher]

Hi guys,

I'm still thinking how to best handle this. What ever solution I come up with, it has it's drawbacks.

Any solution is this form

public var rx_nonMarkedText: ControlProperty<String>
public var rx_markedText: ControlProperty<String>

• Seems kind of weird and complex to me.
• What happens when you subscribe initially, and there is already marked text? The API should always immediately return initial value.
• The observer part doesn't make much sense because the behavior will be either magical or too complex depending on different selection and marked text cases.

I've thought about solutions in this form

public var rx_textAndMarkedRange: ControlProperty<(text: String, markedRange: UITextRange?)>

• but you can't figure out anything about range without UITextField because UITextRange is just an abstract base.

Then I've thought about solutions in this form

public var rx_textAndMarkedRange: ControlProperty<(text: String, markedRange: NSRange?)>

• it could probably be done, but all that complexity just for this use case, and what if you want to change selection also, that would also make sense.
• once you go this path, it kind of explodes in possible API forms

Then I thought, well ok, why don't we just do

public var rx_textFromContainer: ControlProperty<UITextInput>

• But that is also kind of ugly, you are sending mutable sequence elements, which we could maybe live with, but setter makes no sense, and it's all so weird

My suggestion would be ...

I'm not too happy with what I'm about to say, but wouldn't it make sense if we would just add that convenience in front of <-> operator.

    protocol RxTextInput {
         var rx_text: ControlProperty<String> { get }
    }
    extension UITextField : RxTextInput {
    }
    extension UITextView: RxTextInput {
    }
    extension RxTextInput {
    /**
    Reactive wrapper for `text` property. Notifications are fired only when when there
    isn't any marked text.
    */
    public var rx_nonMarkedText: ControlProperty<String> {
        let textProperty = self.rx_text
        return ControlProperty(
            values: textProperty.asObservable().filter { [weak self] _ in self?.markedTextRange == nil },
            valueSink: textProperty.asObserver()
        )
    }
    }

// Two way binding operator between control property and variable, that's all it takes (well, almost) {

infix operator <-> {
}

func <-> <T>(property: ControlProperty<T>, variable: Variable<T>) -> Disposable {
    let bindToUIDisposable = variable.asObservable()
        .bindTo(property)
    let bindToVariable = property
        .subscribe(onNext: { n in
            variable.value = n
        }, onCompleted:  {
            bindToUIDisposable.dispose()
        })

    return StableCompositeDisposable.create(bindToUIDisposable, bindToVariable)
}

// }

We don't bundle that <-> operator anyway, so if you are copy pasting, it's not too hard to copy paste a couple of lines more.

What we could include in the RxCocoa project is maybe just this part

    protocol RxTextInput {
         var rx_text: ControlProperty<String> { get }
    }
    extension UITextField : RxTextInput {
    }
    extension UITextView: RxTextInput {
    }

so you would just need to write

extension RxTextInput {
    /**
    Reactive wrapper for `text` property. Notifications are fired only when when there
    isn't any marked text.
    */
    public var rx_nonMarkedText: ControlProperty<String> {
        let textProperty = self.rx_text
        return ControlProperty(
            values: textProperty.asObservable().filter { [weak self] _ in self?.markedTextRange == nil },
            valueSink: textProperty.asObserver()
        )
    }
    }

// Two way binding operator between control property and variable, that's all it takes (well, almost) {

infix operator <-> {
}

func <-> <T>(property: ControlProperty<T>, variable: Variable<T>) -> Disposable {
    let bindToUIDisposable = variable.asObservable()
        .bindTo(property)
    let bindToVariable = property
        .subscribe(onNext: { n in
            variable.value = n
        }, onCompleted:  {
            bindToUIDisposable.dispose()
        })

    return StableCompositeDisposable.create(bindToUIDisposable, bindToVariable)
}

// }

[omochi]

@kzaher

There are 2 major problems.
One is not solved at all.

First, Japanese developers need to handle this issue.
Yes, we can address it with various ways such like shown in this thread.
In this angle, your opinion is good to keep things simply.
But, there is a lot of document about usage of RxSwift in internet written by Non-Japanese developers.
They don't know about this issue so documents may include this issue.
Some Japanese developer will take their time to fight and solve it.

Second, Japanese user of application made with RxSwift by Non-Japanese developers can not use it correctly.
This issue of input process is very critical as user can not input they want at all.
Please imagine if this happen,
Japanese users may report issue to developer of app.
But it is difficult to understand and debug about problem for Non-Japanese developer (they may does not know about input conversion at all) and also difficult to report problem for Japanese non-developer users with English.
If one developer understand topic and solve it, others does not relate.
So this hard report and handle story may occurs multiple places for each apps.
So we need solve this at root of this, in RxSwift itself. not in individual app developers.

Sorry for my lack of best solution proposal and many negative comments.
This thread looks at first problem mainly.
But I think this second problem is more important than first actually.
Because it has possibility that cause more unhappiness.

[kzaher]

Hi @omochi ,

Thnx for expressing your concerns. I think you are right and we haven't considered problem 2 sufficiently.

Yes, it looks like not only Japanese developers need to take care of that, but all developers. Because if you have app built in Croatia that some Japanese user is using with that type of keyboard, it will fail for them also.

We also agree that I would like to improve situation regarding IME methods.

I don't think that we can change behavior of rx_text just by adding marked text filters like I've tried to explain before:

• it would mean that we need to emit same text values when user is inputing text for every step (which would be extremely weird API behavior, we would get tons of bug reports and PRs for that)
• we can't remedy those different elements by adding distinctUntilChanged because that might actually
  be wanted behavior (like when dealing with validations, that's the reason why we've currently removed it). The knowledge of what to do is simply contained in the particular API consumer outside of the system.
• we can't add additional filtering for getting or setting because if we do that then we'll get bug reports and PRs to make implementation consistent with the name, which it wouldn't be.
• the implementation is also non trivial because we would also need to somehow handle cases where subscription is being made in the middle of editing.
• the real problem actually isn't in rx_text, but in two way binding layer. If two way binding code was written in correct way, then there is no issue. If developer uses this API without two way binding written this way, it will work as far as I can tell.
• it breaks backwards compatibility in a significant way

I think there are two different problems that we should answer.

• How would ideal API look like
• How to educate users to use it correctly

I don't currently see any way how to solve these two points with any solution, no matter how complex.

What I was maybe thinking of doing is:

• improve that binding example and binding operator by adding binding operator (<->) that works on RxTextInput
• add warning logging or fatalError to normal variant (func <-> <T>(property: ControlProperty<T>, variable: Variable<T>) -> Disposable) in case trying to bind to String to use the other version instead
• document all of this in example app

I'm not sure it is possible for us to control what people write on the internet, or create an API so that how ever people use it, it just works in all cases.

I'm open to suggestions as always, but currently I don't see how we can solve this in the rx_text extension itself.

[kzaher]

This should work for second part of the problem :)

extension UITextField {

    /**
    Reactive wrapper for `text` property.
    */
    public var rx_text: ControlProperty<String> {
        return UIControl.rx_value(
            self,
            getter: { textField in
                textField.text ?? ""
            }, setter: { textField, value in
                if textField.text != value {
                    textField.text = value
                }
            }
        )
    }
}

[kzaher]

Changes that fix this problem have been release. Feel free to reopen this issue if needed.

[apinhopt]

Hey there guys,

I have the same issue when binding to a BehaviourRelay. The Japanese highlighting stops working.

let value: BehaviorRelay<String> = .init(value: "")

self.textField.textField?.rx.text.orEmpty
            .bind(to: self.value)
            .disposed(by: self.disposeBag)

//same for this
 self.textField.textField?.rx.text.orEmpty.asDriver()
            .drive(onNext: { (value) in
                self.value.accept(value)
        })
        .disposed(by: disposeBag)