Re-examine the risks of JDK-8264770 breaking third party libraries and applications.

[Nir Lisker]

>
> This actually isn't an issue because adding an invalidation listener
> revalidates the property as well (I'm not sure why, but I noticed this
> before while working on fluent bindings).
>

There is an open issue on this:
https://bugs.openjdk.java.net/browse/JDK-8208750.

This is also the cause for the When binding evaluating the 'false' branch
unneededly. Maybe this is a good time to have a look if this can be
changed. The problem is finding how much code relies on this unspecified
behavior.

On Sun, Aug 29, 2021 at 12:01 AM John Hendrikx <hjohn at xs4all.nl> wrote:

> This actually isn't an issue because adding an invalidation listener
> revalidates the property as well (I'm not sure why, but I noticed this
> before while working on fluent bindings).
>
> However, there is another issue. When I recreate a bidirectional with
> two invalidation listeners with special logic to ignore my own
> invalidation listener triggering again when the other property gets
> updated.
>
> When you include this logic (using the "updating" field in the current
> code) in combination with invalidation listeners, the field that was
> just updated does NOT get revalidated.  Any further changes to that
> field will therefore not trigger an invalidation event and the
> bidirectional nature of the binding breaks.
>
> Try this example:
>
> import javafx.application.Application;
> import javafx.beans.InvalidationListener;
> import javafx.beans.Observable;
> import javafx.beans.property.SimpleDoubleProperty;
> import javafx.stage.Stage;
>
> public class TestBug8264770 extends Application {
>
>    public static void main(String[] args) {
>      Application.launch(args);
>    }
>
>    static SimpleDoubleProperty p1 = new SimpleDoubleProperty(2);
>    static SimpleDoubleProperty p2 = new SimpleDoubleProperty(3);
>
>    @Override
>    public void start(Stage stage) throws Exception {
>      InvalidationListener invalidationListener = obs -> invalidated(obs);
>
>      p1.addListener(invalidationListener);
>      p2.addListener(invalidationListener);
>
>      p1.setValue(4);
>      p2.setValue(5);
>
>      // Prints p1 = 4.0 (!!)
>      System.out.println("Expect p1 to be 5, but was: " + p1.getValue());
>    }
>
>    private boolean updating = false;
>
>    private void invalidated(Observable source) {
>      if (!updating) {
>        try {
>          updating = true;
>
>          if(source == p1) {
>            System.out.println("Setting p2 to " + p1.get());
>            p2.set(p1.get());
>          }
>          else {
>            System.out.println("Setting p1 to " + p2.get());
>            p1.set(p2.get());
>          }
>        }
>        finally {
>          updating = false;
>        }
>      }
>    }
> }
>
> --John
>
> On 28/08/2021 22:14, John Hendrikx wrote:
> > Was it taken into account that when you register an invalidation
> listener:
> >
> >         property1.setValue(property2.getValue());
> >         property1.addListener(binding);
> >         property2.addListener(binding);
> >
> > ... that if property2 is currently invalid it will not send any further
> > invalidations?
> >
> > Property1 is revalidated (because of the getValue call), but property2
> > (if already invalid) will not become valid with that setValue call.
> >
> > Before when they were ChangeListeners, both properties became valid
> > after a bidirectional binding, now that doesn't seem to be the case
> > anymore.
> >
> > --John
> >
> >
> > On 26/08/2021 09:29, Frederic Thevenet wrote:
> >> Hi,
> >>
> >> A change was introduced In JDK-8264770 that swaps the use of
> >> ChangeListeners to InvalidationListeners within the internal
> >> implementation of BidirectionalBinding [1].
> >>
> >> While this change shouldn't normally affect third party applications, it
> >> turned out to break the scrolling facilities used by the widely used
> >> rich text widget RichTextFX [2].
> >>
> >> After a short investigation, I believe the root cause lies within
> >> another library by the same author called ReactFX [3], which aims to
> >> bring reactive programming techniques to JavaFX; in order to do so it
> >> seems to expands on but also sometime overrides the built-in bindings
> >> and events mechanisms.
> >>
> >> Now, I do believe that this is probably an exceptional case, and it is
> >> also quite possible that it  is the result of an unsafe/incorrect use of
> >> internal implementations by the third party library, but with that said
> >> I can't help but feeling a bit nervous about the wider implications of
> >> that change with regard to compatibility of existing apps and OpenJFX
> >> 17. At the very least I believe it is important to raise awareness about
> >> potential compatibility issues among the community.
> >>
> >> For your information, I reached out to the maintainers of RichTextFX and
> >> proposed a workaround (replacing the use of a bidirectional binding by a
> >> pair of explicit ChangeListeners), which, while not very elegant, seems
> >> to fix the particular issue I discovered [4], but unfortunately it seems
> >> development on the underlying ReactFX is no longer active (last commit
> >> in 2016).
> >>
> >> -- Fred
> >>
> >> [1] https://bugs.openjdk.java.net/browse/JDK-8264770
> >>
> >> [2] https://github.com/FXMisc/RichTextFX
> >>
> >> [3] https://github.com/ReactFX/reactfx.github.io
> >>
> >> [4] https://github.com/FXMisc/Flowless/issues/97
> >>
> >
>