JDK-8089557 : bindBidirection works for ReadOnly*Wrapper incorrectly
  • Type: Bug
  • Component: javafx
  • Sub-Component: base
  • Affected Version: 8u31
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-03-11
  • Updated: 2016-08-17
  • Resolved: 2015-08-19
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u72Fixed 9Fixed
Related Reports
Blocks :  
JDK-8132906 - ReadOnly*Wrapper.ReadOnlyPropertyImpl could use ReadOnly*PropertyBase
Duplicate :  
JDK-8147816 - Bindings.bindBidirectional() does not respect read-only wrappers.
Duplicate :  
JDK-8138841 - ReadOnlyBooleanWrapper: Bindings.or & Bindings.and return incorrect results
Relates :  
JDK-8132906 - ReadOnly*Wrapper.ReadOnlyPropertyImpl could use ReadOnly*PropertyBase
Relates :  
JDK-8164096 - ListChangeListener on ReadOnlyListWrapper's getReadOnlyProperty() does not reset change
Description
ReadOnlyStringWrapper a = new ReadOnlyStringWrapper("a");
ReadOnlyStringWrapper b = new ReadOnlyStringWrapper("b");

b.bindBidirectional(a);
//a.bindBidirectional(b); //only this can fix it

a.set("a1"); // a == "a1",b == "a1"
b.set("b1"); // b == "b1", a == "a1" (!!!)

a.set("a2"); // a == "a2",b == "a2"
b.set("b2"); // b == "b2", a == "a2" (!!!)
Comments
Covered by the following unit tests: com/sun/javafx/binding/BidirectionalBindingTest javafx/beans/property/ReadOnlyBooleanWrapperTest javafx/beans/property/ReadOnlyDoubleWrapperTest javafx/beans/property/ReadOnlyFloatWrapperTest javafx/beans/property/ReadOnlyIntegerWrapperTest javafx/beans/property/ReadOnlyListWrapperTest javafx/beans/property/ReadOnlyLongWrapperTest javafx/beans/property/ReadOnlyMapWrapperTest javafx/beans/property/ReadOnlyObjectWrapperTest javafx/beans/property/ReadOnlySetWrapperTest javafx/beans/property/ReadOnlyStringWrapperTest
21-10-2015
I agree. Approved to backport to 8u-dev.
08-10-2015
Kevin, the only visible change of behavior I could think of is that observable value passed to the listener's changed method will be different. That is, before the fix this value would be equal to the value returned from the getReadOnlyProperty and after the fix it will be equal to the wrapper property itself. While this could theoretically break someone's code I think it's better to have this value equals to the observable value to which this listener was added after all.
08-10-2015
@Vadim: is there a chance of a user-visible behavioral regression with this fix? It seems safe enough, but since this bug isn't a regression, I would hate to fix it only to introduce a bug.
08-10-2015
Changeset: 155ad1e34c44 Author: vadim Date: 2015-08-19 13:46 +0300 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/155ad1e34c44
19-08-2015
+1. Looks good to me.
19-08-2015
Chien, please review this fix: http://cr.openjdk.java.net/~vadim/8089557/webrev.00/ ReadOnlyWrappers were simplified so they do not forward add/removeListener calls to the ReadOnlyProperty. This actually make much more sense, so you can attach distinct listeners to any property and they will receive synchronized events. This is checked in the existing tests, so they will fail if fireValueChangedEvent do not pass the event to either super or readOnlyProperty. Changes in the ReadOnlyWrapperTests reflect the fact that listener will get respective observable. BidirectionalBindingTest was greatly simplified and ReadOnlyWrappers were added to the parameters.
04-08-2015
BidirectionalBinding makes an assumption that the property passed in the changed method is the same as was used in the addListener, but the ReadOnly*Wrapper actually pass all calls to the ReadOnlyPropertyImpl. So the source of the changed() call is neither property1 nor property2 and the binding is only works from property2 to property1 by accident.
03-08-2015