[Bug] Replace selected text with pasted text

[obcardinal]

Hi, I came across a problem. If you select the text (let's say 5 characters) and paste text (Ctrl+V) that is longer than the selected text, you get an error:

Exception in thread "JavaFX Application Thread" java.lang.IllegalArgumentException
	at javafx.controls/javafx.scene.control.IndexRange.<init>(IndexRange.java:53)
	at org.fxmisc.richtext.SelectionImpl.selectRange(SelectionImpl.java:312)
	at org.fxmisc.richtext.SelectionImpl.lambda$new$7(SelectionImpl.java:276)
	at org.reactfx.util.NonAccumulativeStreamNotifications.lambda$head$0(NotificationAccumulator.java:134)
	at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:68)
	at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:57)
	at org.reactfx.ProperEventStream.emit(ProperEventStream.java:18)
	at org.reactfx.FilterStream.lambda$observeInputs$0(FilterStream.java:20)
	at org.reactfx.util.NonAccumulativeStreamNotifications.lambda$head$0(NotificationAccumulator.java:134)
	at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:68)
	at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:57)
	at org.reactfx.ProperEventStream.emit(ProperEventStream.java:18)
	at org.reactfx.MappedStream.lambda$observeInputs$0(MappedStream.java:25)
	at org.reactfx.util.QueuingStreamNotifications.lambda$head$0(NotificationAccumulator.java:217)
	at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:68)
	at org.reactfx.SuspendableBase.resume(SuspendableBase.java:64)
	at org.reactfx.CloseableOnceGuard.close(Guard.java:49)
	at org.reactfx.MultiGuard.close(Guard.java:83)
	at org.reactfx.Suspendable$1.resumeSource(Suspendable.java:118)
	at org.reactfx.Suspendable$1.suspendSource(Suspendable.java:104)
	at org.reactfx.util.NonAccumulativeStreamNotifications.lambda$head$0(NotificationAccumulator.java:134)
	at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:68)
	at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:57)
	at org.reactfx.ProperEventStream.emit(ProperEventStream.java:18)
	at org.reactfx.EventStreams$3.lambda$observeInputs$0(EventStreams.java:105)
	at org.reactfx.value.ChangeListenerWrapper.accept(Val.java:786)
	at org.reactfx.util.AbstractReducingStreamNotifications.lambda$head$0(NotificationAccumulator.java:248)
	at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:68)
	at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:57)
	at org.reactfx.value.ValBase.invalidate(ValBase.java:32)
	at org.reactfx.SuspendableBoolean.release(SuspendableBoolean.java:24)
	at org.reactfx.CloseableOnceGuard.close(Guard.java:49)
	at org.reactfx.Suspendable.suspendWhile(Suspendable.java:49)
	at org.fxmisc.richtext.model.GenericEditableStyledDocumentBase.updateMulti(GenericEditableStyledDocumentBase.java:215)
	at org.fxmisc.richtext.model.GenericEditableStyledDocumentBase.updateSingle(GenericEditableStyledDocumentBase.java:207)
	at org.reactfx.util.Tuple3.exec(Tuple3.java:43)
	at org.fxmisc.richtext.model.GenericEditableStyledDocumentBase.replace(GenericEditableStyledDocumentBase.java:142)
	at org.fxmisc.richtext.model.SimpleEditableStyledDocument.replace(SimpleEditableStyledDocument.java:10)
	at org.fxmisc.richtext.GenericStyledArea.replace(GenericStyledArea.java:1195)
	at org.fxmisc.richtext.GenericStyledArea.replaceText(GenericStyledArea.java:1182)
	at org.fxmisc.richtext.TextEditingArea.replaceText(TextEditingArea.java:390)
	at org.fxmisc.richtext.EditActions.replaceSelection(EditActions.java:165)
	at org.fxmisc.richtext.ClipboardActions.paste(ClipboardActions.java:111)

[obcardinal]

I was able to fix the problem if you add this condition:

if (finalEnd < endOfChange) {
    finalEnd = endOfChange;
} else {

RichTextFX/richtextfx/src/main/java/org/fxmisc/richtext/SelectionImpl.java

Lines 251 to 262 in 17a7957

	if (indexOfChange == finalStart && netLength > 0) {
	finalStart = finalStart + netLength;
	} else if (indexOfChange < finalStart) {
	finalStart = finalStart < endOfChange
	? indexOfChange
	: finalStart + netLength;
	}
	if (indexOfChange < finalEnd) {
	finalEnd = finalEnd < endOfChange
	? indexOfChange
	: finalEnd + netLength;
	}

}

Please check with someone.
I don't get any exceptions.
Thanks.

[Jugen]

Hi @MrChebik, I've had a closer look at the proposed fix and also done some debugging. From what I've seen the problem isn't that finalEnd is smaller than endOfChange but rather that finalStart is made larger than finalEnd. So I think that the fix should rather be (insert at line 263 above):

if (finalStart > finalEnd) {
     finalStart = finalEnd;
}

Regards

[obcardinal]

Hi, @Jugen, then, we can simply add the condition:

endOfChange <= finalEnd

And we get something of this:

if (endOfChange <= finalEnd &&
        netLength > 0 &&
        indexOfChange == finalStart) {
    finalStart = finalStart + netLength;
} else if (indexOfChange < finalStart) {
    finalStart = finalStart < endOfChange
            ? indexOfChange
            : finalStart + netLength;
}
if (indexOfChange < finalEnd) {
    finalEnd = finalEnd < endOfChange
            ? indexOfChange
            : finalEnd + netLength;
}

[Jugen]

Yes, I suppose so and I see your commit passed the tests .... but don't you think it would be better to have the check as I suggested because we have three if blocks that can change either finalStart or finalEnd ..... so checking them after seems like a good thing to do ?

[obcardinal]

Ok

[proudsalopian]

A quick fix would be to put a final guard in SelectionImpl at line 276 before calling selectRange to uphold the contract that the calculated selection never ends before it begins:

            finalEnd = Math.max(finalStart,finalEnd); // the selection guard
            selectRange(finalStart, finalEnd);