Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8246745

ListCell/Skin: misbehavior on switching skin

XMLWordPrintable

      for details see comments in the code snippet below, which is the causing all of them. Basically it's trying to wire a path listener to the listView's fixedCellSize (to update internal state of the skin).

          private void setupListeners() {
              ListView listView = getSkinnable().getListView();
              // memory leak if cell never attached to a listView
              if (listView == null) {
                  getSkinnable().listViewProperty().addListener(new InvalidationListener() {
                      @Override public void invalidated(Observable observable) {
                          // a one-time listener for null -> !null listView
                          // so neither catches listViewA -> listViewB nor -> null
                          getSkinnable().listViewProperty().removeListener(this);
                          setupListeners();
                      }
                  });
              } else {
                  // note: this is the listView (A) at the moment of installing the listener
                  this.fixedCellSize = listView.getFixedCellSize();
                  this.fixedCellSizeEnabled = fixedCellSize > 0;
                  registerChangeListener(listView.fixedCellSizeProperty(), e -> {
                      // 1. note: this is the cell's listView at the time of changing
                      // the property of the above - which might not be the same as (A)!
                      // being, introduces false green in tests
                      // 2. throws npe if cell's listView reset to null
                      this.fixedCellSize = getSkinnable().getListView().getFixedCellSize();
                      this.fixedCellSizeEnabled = fixedCellSize > 0;
                  });
              }
          }

      Technical fix is to implement a reliable pattern for path listening.

      But do we really need that internal state? The listener itself does not trigger any action (like f.i. a relayout), it's just setting two fields that are used in computing the xxSizes. These might as well access the current state (extracted to private methods to relieve the computation code from null checks).

      Similar pattern in Tree/Table/Row/Skin and TreeCell/Skin (not in Tree/Table/Cell/Skin)

            fastegal Jeanette Winzenburg
            fastegal Jeanette Winzenburg
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: