United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-4234793 : PopupMenuListener popupMenuCanceled is never called

Details
Type:
Bug
Submit Date:
1999-05-03
Status:
Resolved
Updated Date:
2002-09-28
Project Name:
JDK
Resolved Date:
2002-09-28
Component:
client-libs
OS:
generic
Sub-Component:
javax.swing
CPU:
generic
Priority:
P4
Resolution:
Fixed
Affected Versions:
1.1.7,1.2.0,1.2.2
Fixed Versions:
1.4.2 (mantis)

Related Reports
Duplicate:
Duplicate:

Sub Tasks

Description

Name: vi73552			Date: 05/03/99


import javax.swing.*;
import javax.swing.event.*;
import java.awt.*;
import java.awt.event.*;

class PopupBug extends JFrame
{
    public static void main(String[] args)
    {
        PopupBug a = new PopupBug();
        a.setSize(600, 300);
        a.addWindowListener(
            new WindowAdapter()
            {
                public void windowClosing(WindowEvent evt)
                {
                    System.exit(0);
                }
            }
        );
        a.setVisible(true);
    }
    
    class foo extends AbstractAction
    {
        foo()
        {
            super("foo");
        }
        public void actionPerformed(ActionEvent evt) 
        {
            text.append("action fired\n");
        }
    }
        
    private JTextArea text;
    PopupBug()
    {
        text = new JTextArea();
        Container content = getContentPane();
        content.setLayout(new BorderLayout());
        content.add(new JScrollPane(text), BorderLayout.CENTER);
        JTextArea desc = new JTextArea();
        desc.append("Right Click to display popup menu in text area\n");
        desc.append("Cancel menu by clicking outside menu\n");
        desc.append("Note that *popupMenuwillBecomeInvisible* is called instead of *popupMenuCanceled*");
        content.add(desc, BorderLayout.SOUTH);
        text.addMouseListener(new PopupMouseAdapter());
    }
        
    class PopupMouseAdapter extends MouseAdapter
    {
        private void checkPopup(MouseEvent evt)
        {            
            if (evt.isPopupTrigger())
            {
                int x = evt.getX();
                int y = evt.getY();
                PopupMenuListener ml = new PopupMenuListener()
                {
                    public void popupMenuWillBecomeVisible(PopupMenuEvent e)
                    {
                    }
                    public void popupMenuWillBecomeInvisible(PopupMenuEvent e)
                    {
                        text.append("in popupMenuWillBecomeInvisible\n");
                    }
                    public void popupMenuCanceled(PopupMenuEvent e)
                    {
                        text.append("in popupMenuCanceled\n");
                    }
                };
                JPopupMenu m = new JPopupMenu();
                //m.setLightWeightPopupEnabled(false);
                m.addPopupMenuListener(ml);                        
                m.add(new foo());                
                m.show((Component) evt.getSource(), x, y); 
            }
        }
        public void mousePressed(MouseEvent evt)
        {
            checkPopup(evt);
        }
        public void mouseReleased(MouseEvent evt)
        {
            checkPopup(evt);
        }
        public void mouseClicked(MouseEvent evt)
        {
            checkPopup(evt);
        }
    }
}
(Review ID: 57353) 
======================================================================

                                    

Comments
CONVERTED DATA

BugTraq+ Release Management Values

COMMIT TO FIX:
generic
mantis

FIXED IN:
mantis

INTEGRATED IN:
mantis
mantis-b03


                                     
2004-06-14
PUBLIC COMMENTS

.
                                     
2004-06-10
EVALUATION

The problem is that the BasicPopupMenuUI doens't call JPopupMenu.firePopupMenuCancelled. Previously, the cancelPopupMenu() code and logic to cancel the Popup was in JPopupMenu but was moved into BasicPopupMenuUI. Since firePopupMenuCancelled() is protected, BasicPopupMenuUI has no way of calling this method thereby sending out the popupMenuCancelled() notification.

The solution is to make the firePopupMenuCancelled() method public or add a public or package private method which will call the protected firePopupMenuCancelled() method.

mark.davidson@Eng 2000-03-14


This looks trickier than I thought. The code was refactored so that the MouseGrabber was taken out of JPopupMenu and put into BasicPopupMenuUI. The MouseGrabber.cancelPopupMenu() method should call the JPopupMenu.firePopupMenuCancelled(). This should be done after the statement lastGrapped==getFirstPopup. I'm not too keen on this since it looks a bit messy and I'll have to change firePopupMenuCanceled() from protected to public - which is inconsistent with the other JPopupMenu methods. Still thinking about it.

mark.davidson@Eng 2000-03-28

I tried to add the firePopupMenuCancelled() call in MouseGrabber.cancelPopupMenu() and found that it worked with the current code. However, it's still a hack and not very resiliant to change in complementary classes. It NPE'd when I was refactoring JComboBox. I'm going to abandon this tack and a more drastic change should be thought out. Pehaps with the instroduction of a new interface.
mark.davidson@Eng 2000-04-03

Now that 1.4 and 1.4.1 are out the door and stable, I revisited making JPopupMenu.firePopupMenuCanceled() public and called from the MouseGrabber and the CancelAction (ESCAPE key). This seems to work correctly. The semantics should be that the PopupMenuCanceled event should be before the PopupMenuWillBecomeInvisible event. Also, should ensure that the JComboBox also sends the canceled event when the ESCAPE key is pressed. The diffs are in the suggested fix section.

Unfortunately, we cannot make an API change until 1.5 so I'm going to try another approach by using a client property.


###@###.### 2002-09-04
                                     
2002-09-04
SUGGESTED FIX

Change acces of firePopupMenuCancelled to public. Call within MouseGrabber and CancelAction.


------- JPopupMenu.java -------
663d662
<
668c667
<     protected void firePopupMenuCanceled() {
---
>     public void firePopupMenuCanceled() {


------- BasicComboBoxUI.java -------
1398a1399
>               comboBox.firePopupMenuCanceled();

------- BasicPopupMenuUI.java -------
199a200,212
>     private static MenuElement getFirstPopup() {
>       MenuSelectionManager msm = MenuSelectionManager.defaultManager();
>       MenuElement[] p = msm.getSelectedPath();
>       MenuElement me = null;
>
>       for(int i = 0 ; me == null && i < p.length ; i++) {
>           if (p[i] instanceof JPopupMenu)
>               me = p[i];
>       }
>
>       return me;
>     }
>
238c251,252
<           if (lastGrabbed==getFirstPopup()) {
---
>           MenuElement firstPopup = getFirstPopup();
>           if (lastGrabbed == firstPopup) {
240a255,259
>
>               if (firstPopup instanceof JPopupMenu) {
>                   ((JPopupMenu)firstPopup).firePopupMenuCanceled();
>               }
>
243c262
<               lastGrabbed=getFirstPopup();
---
>               lastGrabbed = firstPopup;
301,313d319
<       private MenuElement getFirstPopup() {
<           MenuSelectionManager msm = MenuSelectionManager.defaultManager();
<           MenuElement[] p = msm.getSelectedPath();
<           MenuElement me = null;
<
<           for(int i = 0 ; me == null && i < p.length ; i++) {
<               if (p[i] instanceof JPopupMenu)
<                   me = p[i];
<           }
<
<           return me;
<       }
<
500d505
<
501a507,510
>           JPopupMenu firstPopup = (JPopupMenu)getFirstPopup();
>           if (firstPopup != null) {
>               firstPopup.firePopupMenuCanceled();
>           }
502a512
>

###@###.### 2002-09-04

Next approach: Add a client property.

This creates an ArrayIndexOutOfBoundsException becaus some dead code becomes enabled. I have just submitted a new bug on this (4741951).

------- BasicComboBoxUI.java -------
1398a1399
>               comboBox.firePopupMenuCanceled();

------- BasicPopupMenuUI.java -------
198a199,210
>     private static MenuElement getFirstPopup() {
>       MenuSelectionManager msm = MenuSelectionManager.defaultManager();
>       MenuElement[] p = msm.getSelectedPath();
>       MenuElement me = null;
>
>       for(int i = 0 ; me == null && i < p.length ; i++) {
>           if (p[i] instanceof JPopupMenu)
>               me = p[i];
>       }
>
>       return me;
>     }
238c250,255
<           if (lastGrabbed==getFirstPopup()) {
---
>           JPopupMenu firstPopup = (JPopupMenu)getFirstPopup();
>           if (lastGrabbed == firstPopup) {
>               // 4234793: This action should call firePopupMenuCanceled but it's
>               // a protected method. The real solution would be to make
>               // firePopupMenuCanceled public and call it directly.
>               firstPopup.putClientProperty("JPopupMenu.firePopupMenuCanceled", Boolean.TRUE);
243c260
<               lastGrabbed=getFirstPopup();
---
>               lastGrabbed = firstPopup;
301,312d317
<       private MenuElement getFirstPopup() {
<           MenuSelectionManager msm = MenuSelectionManager.defaultManager();
<           MenuElement[] p = msm.getSelectedPath();
<           MenuElement me = null;
<
<           for(int i = 0 ; me == null && i < p.length ; i++) {
<               if (p[i] instanceof JPopupMenu)
<                   me = p[i];
<           }
<
<           return me;
<       }
500a506,513
>           // 4234793: This action should call JPopupMenu.firePopupMenuCanceled but it's
>           // a protected method. The real solution would be to make
>           // firePopupMenuCanceled public and call it directly.
>           JPopupMenu firstPopup = (JPopupMenu)getFirstPopup();
>           if (firstPopup != null) {
>               firstPopup.putClientProperty("JPopupMenu.firePopupMenuCanceled", Boolean.TRUE);
>           }
>


------- JPopupMenu.java -------
726a727,736
>
>           // 4234793: Cannot call JPopupMenu.firePopupMenuCanceled since it's
>           // a protected method. The real solution would be to make
>           // firePopupMenuCanceled public and call it directly.
>           Boolean doCanceled = (Boolean)getClientProperty("JPopupMenu.firePopupMenuCanceled");
>           if (doCanceled != null && doCanceled == Boolean.TRUE) {
>               putClientProperty("JPopupMenu.firePopupMenuCanceled", Boolean.FALSE);
>               firePopupMenuCanceled();
>           }
>

This fix can be done for mantis since it doesn't require an API change. However, 4741951 would need to be resolved in conjunction with this fix.




###@###.### 2002-09-04
                                     
2002-09-04



Hardware and Software, Engineered to Work Together