changeset 1262:f848576e5df0

RT-20945: ComboBox, computing the items inside showing property handler fires unwanted onAction event
author jgiles
date Thu, 07 Jun 2012 15:15:05 +1200
parents 60301b7fd01f
children d09cc9910784
files javafx-ui-controls/src/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java javafx-ui-controls/src/javafx/scene/control/ListView.java javafx-ui-controls/src/javafx/scene/control/MultipleSelectionModelBase.java
diffstat 3 files changed, 43 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/javafx-ui-controls/src/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java	Thu Jun 07 13:43:46 2012 +1200
+++ b/javafx-ui-controls/src/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java	Thu Jun 07 15:15:05 2012 +1200
@@ -109,6 +109,7 @@
         super(comboBox, new ComboBoxListViewBehavior<T>(comboBox));
         this.comboBox = comboBox;
         this.listView = createListView();
+        this.textField = getEditableInputNode();
         
         // Fix for RT-21207. Additional code related to this bug is further below.
         this.listView.setManaged(false);
@@ -215,9 +216,6 @@
     @Override public Node getDisplayNode() {
         Node displayNode;
         if (comboBox.isEditable()) {
-            if (textField == null) {
-                textField = getEditableInputNode();
-            }
             displayNode = textField;
         } else {
             displayNode = buttonCell;
--- a/javafx-ui-controls/src/javafx/scene/control/ListView.java	Thu Jun 07 13:43:46 2012 +1200
+++ b/javafx-ui-controls/src/javafx/scene/control/ListView.java	Thu Jun 07 15:15:05 2012 +1200
@@ -58,6 +58,7 @@
 import com.sun.javafx.scene.control.WeakListChangeListener;
 import com.sun.javafx.scene.control.skin.ListViewSkin;
 import com.sun.javafx.scene.control.skin.VirtualContainerBase;
+import java.lang.ref.WeakReference;
 import javafx.beans.DefaultProperty;
 
 /**
@@ -305,19 +306,25 @@
     public final ObjectProperty<ObservableList<T>> itemsProperty() {
         if (items == null) {
             items = new SimpleObjectProperty<ObservableList<T>>(this, "items") {
+                WeakReference<ObservableList<T>> oldItemsRef;
+                
                 @Override protected void invalidated() {
+                    ObservableList<T> oldItems = oldItemsRef == null ? null : oldItemsRef.get();
+                    
                     // FIXME temporary fix for RT-15793. This will need to be
                     // properly fixed when time permits
                     if (getSelectionModel() instanceof ListViewBitSetSelectionModel) {
-                        ((ListViewBitSetSelectionModel)getSelectionModel()).updateItemsObserver(null, getItems());
+                        ((ListViewBitSetSelectionModel)getSelectionModel()).updateItemsObserver(oldItems, getItems());
                     }
                     if (getFocusModel() instanceof ListViewFocusModel) {
-                        ((ListViewFocusModel)getFocusModel()).updateItemsObserver(null, getItems());
+                        ((ListViewFocusModel)getFocusModel()).updateItemsObserver(oldItems, getItems());
                     }
                     if (getSkin() instanceof ListViewSkin) {
                         ListViewSkin skin = (ListViewSkin) getSkin();
                         skin.updateListViewItems();
                     }
+                    
+                    oldItemsRef = new WeakReference<ObservableList<T>>(getItems());
                 }
             };
         }
@@ -907,7 +914,7 @@
 
             this.listView.itemsProperty().addListener(weakItemsObserver);
             if (listView.getItems() != null) {
-                this.listView.getItems().addListener(weakItemsContentObserver);
+                updateItemsObserver(null, listView.getItems());
             }
         }
         
@@ -945,12 +952,14 @@
         private WeakChangeListener weakItemsObserver = 
                 new WeakChangeListener(itemsObserver);
         
+        private int newListHash = -1;
         private void updateItemsObserver(ObservableList<T> oldList, ObservableList<T> newList) {
             // update listeners
             if (oldList != null) {
                 oldList.removeListener(weakItemsContentObserver);
             }
-            if (newList != null) {
+            if (newList != null && newList.hashCode() != newListHash) {
+                newListHash = newList.hashCode();
                 newList.addListener(weakItemsContentObserver);
             }
 
@@ -993,11 +1002,19 @@
             c.reset();
             while (c.next()) {
                 if (c.wasReplaced()) {
-                    // Fix for RT-18969: the list had setAll called on it
-                    int index = getSelectedIndex();
-                    if (index < getItemCount() && index >= 0) {
-                        clearSelection(index);
-                        select(index);
+                    if (c.getList().isEmpty()) {
+                        // the entire items list was emptied - clear selection
+                        clearSelection();
+                    } else {
+                        // Fix for RT-18969: the list had setAll called on it
+                        int index = getSelectedIndex();
+                        if (index < getItemCount() && index >= 0) {
+                            // Use of makeAtomic is a fix for RT-20945
+                            makeAtomic = true;
+                            clearSelection(index);
+                            makeAtomic = false;
+                            select(index);
+                        }
                     }
                 } else if (c.wasAdded() || c.wasRemoved()) {
                     int shift = c.wasAdded() ? c.getAddedSize() : -c.getRemovedSize();
--- a/javafx-ui-controls/src/javafx/scene/control/MultipleSelectionModelBase.java	Thu Jun 07 13:43:46 2012 +1200
+++ b/javafx-ui-controls/src/javafx/scene/control/MultipleSelectionModelBase.java	Thu Jun 07 15:15:05 2012 +1200
@@ -176,10 +176,12 @@
 
     /***********************************************************************
      *                                                                     *
-     * Internal properties                                                 *
+     * Internal field                                                      *
      *                                                                     *
      **********************************************************************/
 
+    // Fix for RT-20945
+    boolean makeAtomic = false;
 
 
     /***********************************************************************
@@ -441,12 +443,16 @@
 
 //            updateLeadSelection();
 //            support.fireChangedEvent(SELECTED_INDICES);
-        selectedIndicesSeq.callObservers(new NonIterableChange.GenericAddRemoveChange<Integer>(0, 0, Collections.singletonList(index), selectedIndicesSeq));
+        selectedIndicesSeq.callObservers(
+                new NonIterableChange.GenericAddRemoveChange<Integer>(0, 0, 
+                Collections.singletonList(index), selectedIndicesSeq));
     }
 
     @Override public void clearSelection() {
-        setSelectedIndex(-1);
-        focus(-1);
+        if (! makeAtomic) {
+            setSelectedIndex(-1);
+            focus(-1);
+        }
 
         if (! selectedIndices.isEmpty()) {
             List<Integer> removed = new AbstractList<Integer>() {
@@ -462,7 +468,10 @@
             };
 
             quietClearSelection();
-            selectedIndicesSeq.callObservers(new NonIterableChange.GenericAddRemoveChange<Integer>(0, 0, removed, selectedIndicesSeq));
+            
+            selectedIndicesSeq.callObservers(
+                    new NonIterableChange.GenericAddRemoveChange<Integer>(0, 0, 
+                    removed, selectedIndicesSeq));
         }
     }
 
@@ -509,4 +518,6 @@
             select(focusIndex + 1);
         }
     }
+    
+    
 }