Skip to content

Commit

Permalink
8176270: Adding ChangeListener to TextField.selectedTextProperty caus…
Browse files Browse the repository at this point in the history 
…es StringOutOfBoundsException

Reviewed-by: aghaisas, kcr
  • Loading branch information
@kevinrushforth
Robert Lichtenberger authored and kevinrushforth committed Jul 13, 2020
1 parent d67c47f commit e2d1c02
Show file tree
Hide file tree
Showing 4 changed files with 202 additions and 48 deletions.
115 changes: 68 additions & 47 deletions modules/javafx.controls/src/main/java/javafx/scene/control/TextInputControl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ protected interface Content extends ObservableStringValue {
public int length();
}

private boolean blockSelectedTextUpdate;

/***************************************************************************
* *
* Constructors *
Expand Down Expand Up @@ -154,21 +156,8 @@ protected TextInputControl(final Content content) {
});

// Bind the selected text to be based on the selection and text properties
selectedText.bind(new StringBinding() {
{ bind(selection, text); }
@Override protected String computeValue() {
String txt = text.get();
IndexRange sel = selection.get();
if (txt == null || sel == null) return "";

int start = sel.getStart();
int end = sel.getEnd();
int length = txt.length();
if (end > start + length) end = length;
if (start > length-1) start = end = 0;
return txt.substring(start, end);
}
});
selection.addListener((ob, o, n) -> updateSelectedText());
text.addListener((ob, o, n) -> updateSelectedText());

focusedProperty().addListener((ob, o, n) -> {
if (n) {
Expand All @@ -184,6 +173,20 @@ protected TextInputControl(final Content content) {
getStyleClass().add("text-input");
}

private void updateSelectedText() {
if (!blockSelectedTextUpdate) {
String txt = text.get();
IndexRange sel = selection.get();
if (txt == null || sel == null) {
selectedText.set("");
} else {
int start = sel.getStart();
int end = sel.getEnd();
selectedText.set(txt.substring(start, end));
}
}
}

/***************************************************************************
* *
* Properties *
Expand Down Expand Up @@ -1139,18 +1142,24 @@ public final void undo() {
final String newText = undoChange.newText;
final String oldText = undoChange.oldText;

if (newText != null) {
getContent().delete(start, start + newText.length(), oldText.isEmpty());
}
blockSelectedTextUpdate = true;
try {
if (newText != null) {
getContent().delete(start, start + newText.length(), oldText.isEmpty());
}

if (oldText != null) {
getContent().insert(start, oldText, true);
doSelectRange(start, start + oldText.length());
} else {
doSelectRange(start, start + newText.length());
}
if (oldText != null) {
getContent().insert(start, oldText, true);
doSelectRange(start, start + oldText.length());
} else {
doSelectRange(start, start + newText.length());
}

undoChange = undoChange.prev;
undoChange = undoChange.prev;
} finally {
blockSelectedTextUpdate = false;
updateSelectedText();
}
}
updateUndoRedoState();
}
Expand All @@ -1168,15 +1177,21 @@ public final void redo() {
final String newText = undoChange.newText;
final String oldText = undoChange.oldText;

if (oldText != null) {
getContent().delete(start, start + oldText.length(), newText.isEmpty());
}
blockSelectedTextUpdate = true;
try {
if (oldText != null) {
getContent().delete(start, start + oldText.length(), newText.isEmpty());
}

if (newText != null) {
getContent().insert(start, newText, true);
doSelectRange(start + newText.length(), start + newText.length());
} else {
doSelectRange(start, start);
if (newText != null) {
getContent().insert(start, newText, true);
doSelectRange(start + newText.length(), start + newText.length());
} else {
doSelectRange(start, start);
}
} finally {
blockSelectedTextUpdate = false;
updateSelectedText();
}
}
updateUndoRedoState();
Expand Down Expand Up @@ -1237,20 +1252,26 @@ private boolean filterAndSet(String value) {
private int replaceText(int start, int end, String value, int anchor, int caretPosition) {
// RT-16566: Need to take into account stripping of chars into the
// final anchor & caret position
int length = getLength();
int adjustmentAmount = 0;
if (end != start) {
getContent().delete(start, end, value.isEmpty());
length -= (end - start);
}
if (value != null) {
getContent().insert(start, value, true);
adjustmentAmount = value.length() - (getLength() - length);
anchor -= adjustmentAmount;
caretPosition -= adjustmentAmount;
}
doSelectRange(anchor, caretPosition);
return adjustmentAmount;
blockSelectedTextUpdate = true;
try {
int length = getLength();
int adjustmentAmount = 0;
if (end != start) {
getContent().delete(start, end, value.isEmpty());
length -= (end - start);
}
if (value != null) {
getContent().insert(start, value, true);
adjustmentAmount = value.length() - (getLength() - length);
anchor -= adjustmentAmount;
caretPosition -= adjustmentAmount;
}
doSelectRange(anchor, caretPosition);
return adjustmentAmount;
} finally {
blockSelectedTextUpdate = false;
updateSelectedText();
}
}

private <T> void updateText(TextFormatter<T> formatter) {
Expand Down
20 changes: 20 additions & 0 deletions modules/javafx.controls/src/test/java/test/javafx/scene/control/TextAreaTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import javafx.scene.Scene;
import javafx.scene.control.TextArea;
import javafx.scene.control.TextInputControlShim;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
Expand All @@ -52,6 +53,25 @@ public class TextAreaTest {
@Before public void setup() {
txtArea = new TextArea();
dummyTxtArea = new TextArea("dummy");
setUncaughtExceptionHandler();
}

@After public void cleanup() {
removeUncaughtExceptionHandler();
}

private void setUncaughtExceptionHandler() {
Thread.currentThread().setUncaughtExceptionHandler((thread, throwable) -> {
if (throwable instanceof RuntimeException) {
throw (RuntimeException)throwable;
} else {
Thread.currentThread().getThreadGroup().uncaughtException(thread, throwable);
}
});
}

private void removeUncaughtExceptionHandler() {
Thread.currentThread().setUncaughtExceptionHandler(null);
}

/*********************************************************************
Expand Down
36 changes: 35 additions & 1 deletion modules/javafx.controls/src/test/java/test/javafx/scene/control/TextFieldTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
import javafx.scene.Scene;
import javafx.scene.control.ComboBox;
import javafx.scene.control.TextField;
import javafx.scene.control.TextFormatter;
import javafx.scene.control.TextFormatter.Change;
import javafx.scene.control.TextInputControlShim;
import javafx.scene.input.KeyCode;
import javafx.scene.input.KeyCodeCombination;
Expand All @@ -70,6 +72,21 @@ public class TextFieldTest {
@Before public void setup() {
txtField = new TextField();
dummyTxtField = new TextField("dummy");
setUncaughtExceptionHandler();
}

private void setUncaughtExceptionHandler() {
Thread.currentThread().setUncaughtExceptionHandler((thread, throwable) -> {
if (throwable instanceof RuntimeException) {
throw (RuntimeException)throwable;
} else {
Thread.currentThread().getThreadGroup().uncaughtException(thread, throwable);
}
});
}

private void removeUncaughtExceptionHandler() {
Thread.currentThread().setUncaughtExceptionHandler(null);
}

/*********************************************************************
Expand Down Expand Up @@ -448,12 +465,28 @@ public void testEnterWithConsumingActionHandler() {
assertTrue("action must be consumed ", actions.get(0).isConsumed());
}

@Test public void replaceSelectionWithFilteredCharacters() {
txtField.setText("x xxxyyy");
txtField.selectRange(2, 5);
txtField.setTextFormatter(new TextFormatter<>(this::noDigits));
txtField.replaceSelection("a1234a");
assertEquals("x aayyy", txtField.getText());
assertEquals(4, txtField.getSelection().getStart());
assertEquals(4, txtField.getSelection().getEnd());
}

private Change noDigits(Change change) {
Change filtered = change.clone();
filtered.setText(change.getText().replaceAll("[0-9]","\n"));
return filtered;
}

/**
* Helper method to init the stage only if really needed.
*/
private void initStage() {
//This step is not needed (Just to make sure StubToolkit is loaded into VM)
Toolkit tk = (StubToolkit)Toolkit.getToolkit();
Toolkit tk = Toolkit.getToolkit();
root = new StackPane();
scene = new Scene(root);
stage = new Stage();
Expand All @@ -465,5 +498,6 @@ public void cleanup() {
if (stage != null) {
stage.hide();
}
removeUncaughtExceptionHandler();
}
}
79 changes: 79 additions & 0 deletions modules/javafx.controls/src/test/java/test/javafx/scene/control/TextInputControlTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
import javafx.scene.control.TextField;
import javafx.scene.control.TextInputControl;
import com.sun.javafx.tk.Toolkit;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -85,6 +87,25 @@ public TextInputControlTest(Class type) {

@Before public void setup() throws Exception {
textInput = (TextInputControl) type.newInstance();
setUncaughtExceptionHandler();
}

@After public void cleanup() {
removeUncaughtExceptionHandler();
}

private void setUncaughtExceptionHandler() {
Thread.currentThread().setUncaughtExceptionHandler((thread, throwable) -> {
if (throwable instanceof RuntimeException) {
throw (RuntimeException)throwable;
} else {
Thread.currentThread().getThreadGroup().uncaughtException(thread, throwable);
}
});
}

private void removeUncaughtExceptionHandler() {
Thread.currentThread().setUncaughtExceptionHandler(null);
}

/******************************************************
Expand Down Expand Up @@ -1865,6 +1886,64 @@ public void caretAndAnchorPositionAfterSettingText() {
assertEquals("", textInput.getText());
}

@Test public void test_redo_replaceText_selectionShortening() {
textInput.setText("0123456789");
assertEquals("0123456789", textInput.getText());

textInput.replaceText(8, 10, "x");
assertEquals("01234567x", textInput.getText());

textInput.undo();
assertEquals("0123456789", textInput.getText());

textInput.redo();
assertEquals("01234567x", textInput.getText());
}

@Test public void replaceSelectionAtEndWithListener() {
StringBuilder selectedTextLog = new StringBuilder();
StringBuilder selectionLog = new StringBuilder();
textInput.setText("x xxx");
textInput.selectRange(2, 5);
textInput.selectedTextProperty().addListener((observable, oldValue, newValue) -> selectedTextLog.append("|" + newValue));
textInput.selectionProperty().addListener((observable, oldValue, newValue) -> selectionLog.append("|" + newValue.getStart() + "," + newValue.getEnd()));
textInput.replaceSelection("a");
assertEquals("|", selectedTextLog.toString());
assertEquals("|3,3", selectionLog.toString());
assertEquals("x a", textInput.getText());
}

@Test public void testSelectionProperties() {
textInput.setText("abcdefghij");

StringBuilder selectedTextLog = new StringBuilder();
StringBuilder selectionLog = new StringBuilder();
StringBuilder textLog = new StringBuilder();
textInput.selectedTextProperty().addListener((observable, oldValue, newValue) -> selectedTextLog.append("|" + newValue));
textInput.selectionProperty().addListener((observable, oldValue, newValue) -> selectionLog.append("|" + newValue.getStart() + "," + newValue.getEnd()));
textInput.textProperty().addListener((observable, oldValue, newValue) -> textLog.append("|" + newValue));

textInput.selectRange(3, 6);
assertEquals("|def", selectedTextLog.toString());
assertEquals("|3,6", selectionLog.toString());
assertEquals("", textLog.toString());

textInput.replaceSelection("xyz");
assertEquals("|def|", selectedTextLog.toString());
assertEquals("|3,6|6,6", selectionLog.toString());
assertEquals("|abcxyzghij", textLog.toString());

textInput.undo();
assertEquals("|def||def", selectedTextLog.toString());
assertEquals("|3,6|6,6|3,6", selectionLog.toString());
assertEquals("|abcxyzghij|abcdefghij", textLog.toString());

textInput.redo();
assertEquals("|def||def|", selectedTextLog.toString());
assertEquals("|3,6|6,6|3,6|6,6", selectionLog.toString());
assertEquals("|abcxyzghij|abcdefghij|abcxyzghij", textLog.toString());
}

// Test for JDK-8178418
@Test public void UndoRedoSpaceSequence() {
Toolkit tk = (StubToolkit)Toolkit.getToolkit();
Expand Down

0 comments on commit e2d1c02

Please sign in to comment.