changeset 6507:c73e9b0c222e

RT-36121: popup should not consume auto-hiding event
author David Grieve<david.grieve@oracle.com>
date Tue, 18 Mar 2014 15:57:24 -0400
parents 99caa949ca39
children dce5642a3cee
files modules/controls/src/main/java/com/sun/javafx/scene/control/behavior/ColorPickerBehavior.java modules/controls/src/main/java/com/sun/javafx/scene/control/behavior/ComboBoxBaseBehavior.java modules/controls/src/main/java/com/sun/javafx/scene/control/behavior/DatePickerBehavior.java modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ColorPickerSkin.java modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxBaseSkin.java modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxPopupControl.java modules/controls/src/main/java/com/sun/javafx/scene/control/skin/DatePickerSkin.java
diffstat 7 files changed, 71 insertions(+), 142 deletions(-) [+]
line wrap: on
line diff
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/behavior/ColorPickerBehavior.java	Tue Mar 18 15:57:19 2014 -0400
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/behavior/ColorPickerBehavior.java	Tue Mar 18 15:57:24 2014 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -96,29 +96,12 @@
     @Override public void onAutoHide() {
         // when we click on some non  interactive part of the 
         // Color Palette - we do not want to hide.
-        wasComboBoxButtonClickedForAutoHide = mouseInsideButton;
         ColorPicker colorPicker = (ColorPicker)getControl();
         ColorPickerSkin cpSkin = (ColorPickerSkin)colorPicker.getSkin();
         cpSkin.syncWithAutoUpdate();
+        // if the ColorPicker is no longer showing, then invoke the super method
+        // to keep its show/hide state in sync.
+        if (!colorPicker.isShowing()) super.onAutoHide();
     }
-    
-    @Override public void mouseReleased(MouseEvent e) {
-        // Overriding to not do the usual on mouseReleased.
-        // The event is handled by the skin instead, which calls
-        // the method below.
-    }
-//    
-    /**
-     * Handles mouse release events.  This will be called by the skin.
-     *
-     * @param e the mouse press event
-     * @param behaveLikeButton if true, this should act just like a button
-     */
-    public void mouseReleased(MouseEvent e, boolean showHidePopup) {
-        if (showHidePopup) {
-            super.mouseReleased(e);
-        } else {
-            disarm();
-        }
-    }
+
 }
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/behavior/ComboBoxBaseBehavior.java	Tue Mar 18 15:57:19 2014 -0400
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/behavior/ComboBoxBaseBehavior.java	Tue Mar 18 15:57:24 2014 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -25,8 +25,11 @@
 
 package com.sun.javafx.scene.control.behavior;
 
+import javafx.event.EventTarget;
 import javafx.scene.Node;
 import javafx.scene.control.ComboBoxBase;
+import javafx.scene.control.Control;
+import javafx.scene.input.KeyEvent;
 import javafx.scene.input.MouseButton;
 import javafx.scene.input.MouseEvent;
 import java.util.ArrayList;
@@ -113,6 +116,12 @@
         COMBO_BOX_BASE_BINDINGS.add(new KeyBinding(ENTER, KEY_RELEASED, RELEASE_ACTION));
     }
 
+    @Override protected void callActionForEvent(KeyEvent e) {
+        // If popup is shown, KeyEvent causes popup to close
+        showPopupOnMouseRelease = true;
+        super.callActionForEvent(e);
+    }
+
     @Override protected void callAction(String name) {
         if (PRESS_ACTION.equals(name)) {
             keyPressed();
@@ -170,52 +179,46 @@
      * Mouse Events                                                           *
      *                                                                        *
      *************************************************************************/
-    
+
     @Override public void mousePressed(MouseEvent e) {
         super.mousePressed(e);
-        getFocus();
         arm(e);
-        wasComboBoxButtonClickedForAutoHide = false;
     }
-    
+
     @Override public void mouseReleased(MouseEvent e) {
-        super.mousePressed(e);
+        super.mouseReleased(e);
 
-        boolean wasArmed = getControl().isArmed();
         disarm();
 
-        // The wasComboBoxButtonClickedForAutoHide boolean was added to resolve
+        // The showPopupOnMouseRelease boolean was added to resolve
         // RT-18151: namely, clicking on the comboBox button shouldn't hide, 
-        // and then immediately show the popup, which was occuring because we 
-        // didn't know where the popup autohide was occurring. Another comment
-        // appears below in the autoHide() method.
-        if (getControl().isShowing()) {
-            hide();
-        } else if (! wasComboBoxButtonClickedForAutoHide 
-                && getControl().contains(e.getX(), e.getY())
-                && e.getButton() == MouseButton.PRIMARY
-                && wasArmed) {
+        // and then immediately show the popup, which was occurring because we
+        // can't know whether the popup auto-hide was coming because of a MOUSE_PRESS
+        // since PopupWindow calls hide() before it calls onAutoHide().
+        if (showPopupOnMouseRelease) {
             show();
         } else {
-            wasComboBoxButtonClickedForAutoHide = false;
+            showPopupOnMouseRelease = true;
+            hide();
         }
     }
 
     @Override public void mouseEntered(MouseEvent e) {
-        if (getControl().isEditable()) {
-            Node arrowButton = getControl().lookup("#arrow-button");
-            mouseInsideButton = arrowButton != null && arrowButton.localToScene(arrowButton.getBoundsInLocal()).contains(e.getSceneX(), e.getSceneY());
+        super.mouseEntered(e);
+
+        if (!getControl().isEditable()) {
+            mouseInsideButton = true;
         } else {
-            mouseInsideButton = true;
+            // This is strongly tied to ComboBoxBaseSkin
+            final EventTarget target = e.getTarget();
+            mouseInsideButton = (target instanceof Node && "arrow-button".equals(((Node) target).getId()));
         }
-        
-        super.mouseEntered(e);
         arm();
     }
 
     @Override public void mouseExited(MouseEvent e) {
+        super.mouseExited(e);
         mouseInsideButton = false;
-        super.mouseExited(e);
         disarm();
     }
     
@@ -246,18 +249,19 @@
             getControl().hide();
         }
     }
-    
-    boolean wasComboBoxButtonClickedForAutoHide = false;
-    boolean mouseInsideButton = false;
+
+    private boolean showPopupOnMouseRelease = true;
+    private boolean mouseInsideButton = false;
     public void onAutoHide() {
-        // if the ComboBox button was clicked, and it was this that forced the
-        // popup to disappear, we don't want the popup to immediately reappear,
-        // so we set wasComboBoxButtonClickedForAutoHide to reflect whether the
-        // mouse was within the comboBox button at the time of autohide occuring.
-        wasComboBoxButtonClickedForAutoHide = mouseInsideButton;
+        // RT-18151: if the ComboBox button was clicked, and it was this that forced the
+        // popup to disappear, we don't want the popup to immediately reappear.
+        // If the mouse was not within the comboBox button at the time of the auto-hide occurring,
+        // then showPopupOnMouseRelease returns to its default of true; otherwise, it toggles.
+        // Note that this logic depends on popup.setAutoHide(true) in ComboBoxPopupControl
         hide();
+        showPopupOnMouseRelease = mouseInsideButton ? !showPopupOnMouseRelease : true;
     }
-    
+
     public void arm() {
         if (getControl().isPressed()) {
             getControl().arm();
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/behavior/DatePickerBehavior.java	Tue Mar 18 15:57:19 2014 -0400
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/behavior/DatePickerBehavior.java	Tue Mar 18 15:57:24 2014 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2014 Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -101,41 +101,15 @@
         }
     }
 
-     /**************************************************************************
-     *                                                                        *
-     * Mouse Events                                                           *
-     *                                                                        *
-     *************************************************************************/
-     /**
-     * When a mouse button is pressed, we either want to behave like a button or
-     * show the popup.  This will be called by the skin.
-     */
     @Override public void onAutoHide() {
         // when we click on some non-interactive part of the
         // calendar - we do not want to hide.
-        wasComboBoxButtonClickedForAutoHide = mouseInsideButton;
         DatePicker datePicker = (DatePicker)getControl();
         DatePickerSkin cpSkin = (DatePickerSkin)datePicker.getSkin();
         cpSkin.syncWithAutoUpdate();
+        // if the DatePicker is no longer showing, then invoke the super method
+        // to keep its show/hide state in sync.
+        if (!datePicker.isShowing()) super.onAutoHide();
     }
 
-    @Override public void mouseReleased(MouseEvent e) {
-        // Overriding to not do the usual on mouseReleased.
-        // The event is handled by the skin instead, which calls
-        // the method below.
-    }
-
-    /**
-     * Handles mouse release events.  This will be called by the skin.
-     *
-     * @param e the mouse press event
-     * @param showHidePopup if true, this should act just like a button
-     */
-    public void mouseReleased(MouseEvent e, boolean showHidePopup) {
-        if (showHidePopup) {
-            super.mouseReleased(e);
-        } else {
-            disarm();
-        }
-    }
 }
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ColorPickerSkin.java	Tue Mar 18 15:57:19 2014 -0400
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ColorPickerSkin.java	Tue Mar 18 15:57:24 2014 -0400
@@ -176,48 +176,12 @@
     public ColorPickerSkin(final ColorPicker colorPicker) {
         super(colorPicker, new ColorPickerBehavior(colorPicker));
         updateComboBoxMode();
-        if (getMode() == ComboBoxMode.BUTTON || getMode() == ComboBoxMode.COMBOBOX) {
-             if (arrowButton.getOnMouseReleased() == null) {
-                arrowButton.setOnMouseReleased(new EventHandler<MouseEvent>() {
-                    @Override public void handle(MouseEvent e) {
-                        ((ColorPickerBehavior)getBehavior()).mouseReleased(e, true);
-                        e.consume();
-                    }
-                });
-            }
-        } else if (getMode() == ComboBoxMode.SPLITBUTTON) {
-            if (arrowButton.getOnMouseReleased() == null) {
-                arrowButton.setOnMouseReleased(new EventHandler<MouseEvent>() {
-                    @Override public void handle(MouseEvent e) {
-                        ((ColorPickerBehavior)getBehavior()).mouseReleased(e, true);
-                        e.consume();
-                    }
-                });
-            }
-        }
         registerChangeListener(colorPicker.valueProperty(), "VALUE");
 
         // create displayNode
         displayNode = new Label();
         displayNode.getStyleClass().add("color-picker-label");
-        if (getMode() == ComboBoxMode.BUTTON || getMode() == ComboBoxMode.COMBOBOX) {
-            if (displayNode.getOnMouseReleased() == null) {
-                displayNode.setOnMouseReleased(new EventHandler<MouseEvent>() {
-                    @Override public void handle(MouseEvent e) {
-                        ((ColorPickerBehavior)getBehavior()).mouseReleased(e, true);
-                    }
-                });
-            }
-        } else {
-            if (displayNode.getOnMouseReleased() == null) {
-                displayNode.setOnMouseReleased(new EventHandler<MouseEvent>() {
-                    @Override public void handle(MouseEvent e) {
-                        ((ColorPickerBehavior)getBehavior()).mouseReleased(e, false);
-                        e.consume();
-                    }
-                });
-            }
-        }
+
         // label graphic
         pickerColorBox = new PickerColorBox();
         pickerColorBox.getStyleClass().add("picker-color");
@@ -234,15 +198,6 @@
 
         pickerColorBox.getChildren().add(colorRect);
         displayNode.setGraphic(pickerColorBox);
-        if (displayNode.getOnMouseReleased() == null) {
-            displayNode.setOnMouseReleased(new EventHandler<MouseEvent>() {
-                @Override
-                public void handle(MouseEvent e) {
-                    ((ColorPickerBehavior)getBehavior()).mouseReleased(e, false);
-                    e.consume();
-                }
-            });
-        }
     }
 
 
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxBaseSkin.java	Tue Mar 18 15:57:19 2014 -0400
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxBaseSkin.java	Tue Mar 18 15:57:24 2014 -0400
@@ -26,12 +26,14 @@
 package com.sun.javafx.scene.control.skin;
 
 import com.sun.javafx.scene.control.behavior.ComboBoxBaseBehavior;
+import javafx.beans.InvalidationListener;
 import javafx.beans.value.ChangeListener;
 import javafx.beans.value.ObservableValue;
 import javafx.geometry.HPos;
 import javafx.geometry.VPos;
 import javafx.scene.Node;
 import javafx.scene.control.ComboBoxBase;
+import javafx.scene.input.MouseEvent;
 import javafx.scene.layout.Region;
 import javafx.scene.layout.StackPane;
 
@@ -58,15 +60,30 @@
         arrow = new Region();
         arrow.setFocusTraversable(false);
         arrow.getStyleClass().setAll("arrow");
+        arrow.setId("arrow");
         arrow.setMaxWidth(Region.USE_PREF_SIZE);
         arrow.setMaxHeight(Region.USE_PREF_SIZE);
+        arrow.setMouseTransparent(true);
+
         arrowButton = new StackPane();
         arrowButton.setFocusTraversable(false);
         arrowButton.setId("arrow-button");
         arrowButton.getStyleClass().setAll("arrow-button");
         arrowButton.getChildren().add(arrow);
+
+        if (comboBox.isEditable()) {
+            //
+            // arrowButton behaves like a button.
+            // This is strongly tied to the implementation in ComboBoxBaseBehavior.
+            //
+            arrowButton.addEventHandler(MouseEvent.MOUSE_ENTERED,  (e) -> getBehavior().mouseEntered(e));
+            arrowButton.addEventHandler(MouseEvent.MOUSE_PRESSED,  (e) -> { getBehavior().mousePressed(e);  e.consume(); });
+            arrowButton.addEventHandler(MouseEvent.MOUSE_RELEASED, (e) -> { getBehavior().mouseReleased(e); e.consume();});
+            arrowButton.addEventHandler(MouseEvent.MOUSE_EXITED, (e) -> getBehavior().mouseExited(e));
+
+        }
         getChildren().add(arrowButton);
-        
+
         // When ComboBoxBase focus shifts to another node, it should hide.
         getSkinnable().focusedProperty().addListener(new ChangeListener<Boolean>() {
             @Override public void changed(ObservableValue<? extends Boolean> observable, Boolean oldValue, Boolean newValue) {
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxPopupControl.java	Tue Mar 18 15:57:19 2014 -0400
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxPopupControl.java	Tue Mar 18 15:57:24 2014 -0400
@@ -110,13 +110,16 @@
                     @Override public void dispose() { }
                 });
             }
+
         };
         popup.getStyleClass().add(COMBO_BOX_STYLE_CLASS);
+        popup.setConsumeAutoHidingEvents(false);
         popup.setAutoHide(true);
         popup.setAutoFix(true);
         popup.setHideOnEscape(true);
         popup.setOnAutoHide(new EventHandler<Event>() {
-            @Override public void handle(Event e) {
+            @Override
+            public void handle(Event e) {
                 getBehavior().onAutoHide();
             }
         });
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/DatePickerSkin.java	Tue Mar 18 15:57:19 2014 -0400
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/DatePickerSkin.java	Tue Mar 18 15:57:24 2014 -0400
@@ -45,6 +45,7 @@
 import javafx.scene.control.DatePicker;
 import javafx.scene.control.TextField;
 import javafx.scene.input.*;
+import javafx.scene.layout.Region;
 import javafx.util.StringConverter;
 
 import com.sun.javafx.scene.control.behavior.DatePickerBehavior;
@@ -69,14 +70,6 @@
             getChildren().add(textField);
         }
 
-        if (arrowButton.getOnMouseReleased() == null) {
-            arrowButton.setOnMouseReleased(new EventHandler<MouseEvent>() {
-                @Override public void handle(MouseEvent e) {
-                    ((DatePickerBehavior)getBehavior()).mouseReleased(e, true);
-                    e.consume();
-                }
-            });
-        }
 
         // The "arrow" is actually a rectangular svg icon resembling a calendar.
         // Round the size of the icon to whole integers to get sharp edges.