Skip to content
Permalink
Browse files
8273138: BidirectionalBinding fails to observe changes of invalid pro… 
…perties

Backport-of: 26d6438
  • Loading branch information
@kevinrushforth
kevinrushforth committed Sep 15, 2021
1 parent 3c6596b commit ff4a6259a877d2cac67b8833c7d297c887bd1fbc
45 modules/javafx.base/src/main/java/com/sun/javafx/binding/BidirectionalBinding.java
View file
@@ -37,6 +37,14 @@
import java.text.ParseException;
import java.util.Objects;

/**
* @implNote Bidirectional bindings are implemented with InvalidationListeners, which are fired once
* when the changed property was valid, but elided for any future changes until the property
* is again validated by calling its getValue()/get() method.
* Since bidirectional bindings require that we observe all property changes, independent
* of whether the property was validated by user code, we manually validate both properties
* by calling their getter method in all relevant places.
*/
public abstract class BidirectionalBinding implements InvalidationListener, WeakListener {

private static void checkParameters(Object property1, Object property2) {
@@ -62,6 +70,7 @@ public static <T> BidirectionalBinding bind(Property<T> property1, Property<T> p
new BidirectionalBooleanBinding((BooleanProperty) property1, (BooleanProperty) property2)
: new TypedGenericBidirectionalBinding<T>(property1, property2);
property1.setValue(property2.getValue());
property1.getValue();
property1.addListener(binding);
property2.addListener(binding);
return binding;
@@ -72,6 +81,7 @@ public static Object bind(Property<String> stringProperty, Property<?> otherProp
Objects.requireNonNull(format, "Format cannot be null");
final var binding = new StringFormatBidirectionalBinding(stringProperty, otherProperty, format);
stringProperty.setValue(format.format(otherProperty.getValue()));
stringProperty.getValue();
stringProperty.addListener(binding);
otherProperty.addListener(binding);
return binding;
@@ -82,6 +92,7 @@ public static <T> Object bind(Property<String> stringProperty, Property<T> other
Objects.requireNonNull(converter, "Converter cannot be null");
final var binding = new StringConverterBidirectionalBinding<>(stringProperty, otherProperty, converter);
stringProperty.setValue(converter.toString(otherProperty.getValue()));
stringProperty.getValue();
stringProperty.addListener(binding);
otherProperty.addListener(binding);
return binding;
@@ -143,6 +154,7 @@ private static <T extends Number> BidirectionalBinding bindNumberObject(Property
final BidirectionalBinding binding = new TypedNumberBidirectionalBinding<>(property2, property1);

property1.setValue(property2.getValue());
property1.getValue();
property1.addListener(binding);
property2.addListener(binding);
return binding;
@@ -154,6 +166,7 @@ private static <T extends Number> BidirectionalBinding bindNumber(Property<T> pr
final BidirectionalBinding binding = new TypedNumberBidirectionalBinding<>(property1, property2);

property1.setValue((T)property2.getValue());
property1.getValue();
property1.addListener(binding);
property2.addListener(binding);
return binding;
@@ -250,18 +263,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
boolean newValue = property1.get();
property2.set(newValue);
property2.get();
oldValue = newValue;
} else {
boolean newValue = property2.get();
property1.set(newValue);
property1.get();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.set(oldValue);
property1.get();
} else {
property2.set(oldValue);
property2.get();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -323,18 +340,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
double newValue = property1.get();
property2.set(newValue);
property2.get();
oldValue = newValue;
} else {
double newValue = property2.get();
property1.set(newValue);
property1.get();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.set(oldValue);
property1.get();
} else {
property2.set(oldValue);
property2.get();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -396,18 +417,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
float newValue = property1.get();
property2.set(newValue);
property2.get();
oldValue = newValue;
} else {
float newValue = property2.get();
property1.set(newValue);
property1.get();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.set(oldValue);
property1.get();
} else {
property2.set(oldValue);
property2.get();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -469,18 +494,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
int newValue = property1.get();
property2.set(newValue);
property2.get();
oldValue = newValue;
} else {
int newValue = property2.get();
property1.set(newValue);
property1.get();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.set(oldValue);
property1.get();
} else {
property2.set(oldValue);
property2.get();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -542,18 +571,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
long newValue = property1.get();
property2.set(newValue);
property2.get();
oldValue = newValue;
} else {
long newValue = property2.get();
property1.set(newValue);
property1.get();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.set(oldValue);
property1.get();
} else {
property2.set(oldValue);
property2.get();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -615,18 +648,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
T newValue = property1.getValue();
property2.setValue(newValue);
property2.getValue();
oldValue = newValue;
} else {
T newValue = property2.getValue();
property1.setValue(newValue);
property1.getValue();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.setValue(oldValue);
property1.getValue();
} else {
property2.setValue(oldValue);
property2.getValue();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -688,18 +725,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
T newValue = property1.getValue();
property2.setValue(newValue);
property2.getValue();
oldValue = newValue;
} else {
T newValue = (T)property2.getValue();
property1.setValue(newValue);
property1.getValue();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.setValue((T)oldValue);
property1.getValue();
} else {
property2.setValue(oldValue);
property2.getValue();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -789,16 +830,20 @@ public void invalidated(Observable observable) {
if (property1 == observable) {
try {
property2.setValue(fromString(property1.getValue()));
property2.getValue();
} catch (Exception e) {
Logging.getLogger().warning("Exception while parsing String in bidirectional binding", e);
property2.setValue(null);
property2.getValue();
}
} else {
try {
property1.setValue(toString(property2.getValue()));
property1.getValue();
} catch (Exception e) {
Logging.getLogger().warning("Exception while converting Object to String in bidirectional binding", e);
property1.setValue("");
property1.getValue();
}
}
} finally {
9 modules/javafx.base/src/test/java/test/com/sun/javafx/binding/BidirectionalBindingTest.java
View file
@@ -301,6 +301,15 @@ public void testDoubleBrokenBind() {
assertEquals(v[1], op2.getValue());
}

@Test
public void testSetValueWithoutIntermediateValidation() {
BidirectionalBinding.bind(op1, op2);
op1.setValue(v[0]);
op2.setValue(v[1]);
assertEquals(v[1], op1.getValue());
assertEquals(v[1], op2.getValue());
}

@Parameterized.Parameters
public static Collection<Object[]> parameters() {
final Boolean[] booleanData = new Boolean[] {true, false, true, false};

1 comment on commit ff4a625

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on ff4a625 Sep 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.