United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-4626632 : Various containers for Action throw NPE when PropChgEvent has null newValue

Details
Type:
Bug
Submit Date:
2002-01-22
Status:
Resolved
Updated Date:
2005-08-31
Project Name:
JDK
Resolved Date:
2005-08-31
Component:
client-libs
OS:
solaris_9,windows_nt,linux,generic,windows_xp,windows_2000
Sub-Component:
javax.swing
CPU:
x86,sparc,generic
Priority:
P4
Resolution:
Fixed
Affected Versions:
1.2.1,1.3.1,1.4.0,1.4.1,1.4.2,5.0
Fixed Versions:

Related Reports
Duplicate:
Duplicate:
Duplicate:
Duplicate:
Duplicate:
Duplicate:
Duplicate:
Duplicate:
Duplicate:
Duplicate:
Relates:

Sub Tasks

Description
Name: jk109818			Date: 01/22/2002


FULL PRODUCT VERSION :
java version "1.4.0-beta3"
Java(TM) 2 Runtime Environment, Standard Edition (build
1.4.0-beta3-b84)
Java HotSpot(TM) Client VM (build 1.4.0-beta3-b84, mixed
mode)

Same problem occurs under 1.3.1_01.

glibc-2.1.3-21
Linux jesse_laptop.netbeans.com 2.2.12-20 #1 Mon Sep 27
10:40:35 EDT 1999 i686 unknown
Red Hat Linux release 6.1 (Cartman)


A DESCRIPTION OF THE PROBLEM :
Various Swing action containers, for example AbstractButton,
JMenu, and JPopupMenu, permit a javax.swing.Action to be
added directly as of 1.3. They listen for property changes
on property "enabled" in order to update action state.
However if the Action implementation fires a
PropertyChangeEvent with a null newValue, these containers
simply throw NPE.

The reason is obvious in their code, e.g. JPopupMenu.java:

} else if (propertyName.equals("enabled")) {
    Boolean enabledState = (Boolean) e.getNewValue();
    menuItem.setEnabled(enabledState.booleanValue());

The code should check for enabledState == null and if so,
use ((Action)e.getSource()).isEnabled() instead. It should
also trigger the clause if propertyName == null. I imagine
there are other such places in the code, I have only noticed
these.

According to the JavaBeans Specification it is acceptable to
use a null newValue (or oldValue, or propertyName) in case
an event source finds it too expensive to compute old and
new values, or is not sure whether the values will actually
be needed by someone but wishes to ensure that a change is
fired nonetheless:

http://java.sun.com/j2se/1.3/docs/api/java/beans/PropertyChangeEvent.html

"Null values may be provided for the old and the new values
if their true values are not known. An event source may send
a null object as the name to indicate that an arbitrary set
of if its properties have changed. In this case the old and
new values should also be null."

I could see no documentation overriding this proviso
specifically for Action's, so it seems that the containers
are at fault.

FYI, bugs and troubles in NetBeans/Forte for Java caused by
this:

http://www.netbeans.org/issues/show_bug.cgi?id=13084
http://openide.netbeans.org/servlets/ReadMsg?msgId=219999&listName=dev


STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Compile and run the enclosed class. When the frame appears,
select the menu item; an NPE is thrown. Select the toolbar
button; ditto. Select the button and when the popup menu
appears, select its item; ditto.

EXPECTED VERSUS ACTUAL BEHAVIOR :
Expected results: all three uses of the action do nothing
and throw no exception.

Actual results: they do nothing but throw exceptions, see
Error Messages section.

ERROR MESSAGES/STACK TRACES THAT OCCUR :
java.lang.NullPointerException
	at
javax.swing.JMenu$ActionChangedListener.propertyChange(JMenu.java:678)
	at
javax.swing.event.SwingPropertyChangeSupport.firePropertyChange(SwingPropertyChangeSupport.java:267)
	at
javax.swing.event.SwingPropertyChangeSupport.firePropertyChange(SwingPropertyChangeSupport.java:235)
	at
javax.swing.AbstractAction.firePropertyChange(AbstractAction.java:181)
	at
TestNullPropChangeNewValuesFromAction.actionPerformed(TestNullPropChangeNewValuesFromAction.java:10)
	at
javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1770)
	at
javax.swing.AbstractButton$ForwardActionEvents.actionPerformed(AbstractButton.java:1823)
	at
javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:422)
	at
javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:260)
	at
javax.swing.AbstractButton.doClick(AbstractButton.java:292)
	at
javax.swing.plaf.basic.BasicMenuItemUI.doClick(BasicMenuItemUI.java:1094)
	at
javax.swing.plaf.basic.BasicMenuItemUI$MenuDragMouseHandler.menuDragMouseReleased(BasicMenuItemUI.java:993)
	at
javax.swing.JMenuItem.fireMenuDragMouseReleased(JMenuItem.java:585)
	at
javax.swing.JMenuItem.processMenuDragMouseEvent(JMenuItem.java:482)
	at
javax.swing.JMenuItem.processMouseEvent(JMenuItem.java:429)
	at
javax.swing.MenuSelectionManager.processMouseEvent(MenuSelectionManager.java:282)
	at
javax.swing.plaf.basic.BasicMenuUI$MouseInputHandler.mouseReleased(BasicMenuUI.java:337)
	at
java.awt.AWTEventMulticaster.mouseReleased(AWTEventMulticaster.java:230)
	at
java.awt.Component.processMouseEvent(Component.java:5020)
	at java.awt.Component.processEvent(Component.java:4819)
	at java.awt.Container.processEvent(Container.java:1383)
	at
java.awt.Component.dispatchEventImpl(Component.java:3527)
	at
java.awt.Container.dispatchEventImpl(Container.java:1440)
	at java.awt.Component.dispatchEvent(Component.java:3368)
	at
java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:3219)
	at
java.awt.LightweightDispatcher.processMouseEvent(Container.java:2930)
	at
java.awt.LightweightDispatcher.dispatchEvent(Container.java:2866)
	at
java.awt.Container.dispatchEventImpl(Container.java:1426)
	at java.awt.Window.dispatchEventImpl(Window.java:1568)
	at java.awt.Component.dispatchEvent(Component.java:3368)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:448)
	at
java.awt.EventDispatchThread.pumpOneEventForHierarchy(EventDispatchThread.java:193)
	at
java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:147)
	at
java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:141)
	at
java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:133)
	at
java.awt.EventDispatchThread.run(EventDispatchThread.java:101)
java.lang.NullPointerException
	at
javax.swing.AbstractButton$ButtonActionPropertyChangeListener.propertyChange(AbstractButton.java:1154)
	at
javax.swing.event.SwingPropertyChangeSupport.firePropertyChange(SwingPropertyChangeSupport.java:267)
	at
javax.swing.event.SwingPropertyChangeSupport.firePropertyChange(SwingPropertyChangeSupport.java:235)
	at
javax.swing.AbstractAction.firePropertyChange(AbstractAction.java:181)
	at
TestNullPropChangeNewValuesFromAction.actionPerformed(TestNullPropChangeNewValuesFromAction.java:10)
	at
javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1770)
	at
javax.swing.AbstractButton$ForwardActionEvents.actionPerformed(AbstractButton.java:1823)
	at
javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:422)
	at
javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:260)
	at
javax.swing.plaf.basic.BasicButtonListener.mouseReleased(BasicButtonListener.java:261)
	at
java.awt.Component.processMouseEvent(Component.java:5020)
	at java.awt.Component.processEvent(Component.java:4819)
	at java.awt.Container.processEvent(Container.java:1383)
	at
java.awt.Component.dispatchEventImpl(Component.java:3527)
	at
java.awt.Container.dispatchEventImpl(Container.java:1440)
	at java.awt.Component.dispatchEvent(Component.java:3368)
	at
java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:3219)
	at
java.awt.LightweightDispatcher.processMouseEvent(Container.java:2930)
	at
java.awt.LightweightDispatcher.dispatchEvent(Container.java:2866)
	at
java.awt.Container.dispatchEventImpl(Container.java:1426)
	at java.awt.Window.dispatchEventImpl(Window.java:1568)
	at java.awt.Component.dispatchEvent(Component.java:3368)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:448)
	at
java.awt.EventDispatchThread.pumpOneEventForHierarchy(EventDispatchThread.java:193)
	at
java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:147)
	at
java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:141)
	at
java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:133)
	at
java.awt.EventDispatchThread.run(EventDispatchThread.java:101)
java.lang.NullPointerException
	at
javax.swing.JPopupMenu$ActionChangedListener.propertyChange(JPopupMenu.java:393)
	at
javax.swing.event.SwingPropertyChangeSupport.firePropertyChange(SwingPropertyChangeSupport.java:267)
	at
javax.swing.event.SwingPropertyChangeSupport.firePropertyChange(SwingPropertyChangeSupport.java:235)
	at
javax.swing.AbstractAction.firePropertyChange(AbstractAction.java:181)
	at
TestNullPropChangeNewValuesFromAction.actionPerformed(TestNullPropChangeNewValuesFromAction.java:10)
	at
javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1770)
	at
javax.swing.AbstractButton$ForwardActionEvents.actionPerformed(AbstractButton.java:1823)
	at
javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:422)
	at
javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:260)
	at
javax.swing.AbstractButton.doClick(AbstractButton.java:292)
	at
javax.swing.plaf.basic.BasicMenuItemUI.doClick(BasicMenuItemUI.java:1094)
	at
javax.swing.plaf.basic.BasicMenuItemUI$MouseInputHandler.mouseReleased(BasicMenuItemUI.java:934)
	at
java.awt.Component.processMouseEvent(Component.java:5020)
	at java.awt.Component.processEvent(Component.java:4819)
	at java.awt.Container.processEvent(Container.java:1383)
	at
java.awt.Component.dispatchEventImpl(Component.java:3527)
	at
java.awt.Container.dispatchEventImpl(Container.java:1440)
	at java.awt.Component.dispatchEvent(Component.java:3368)
	at
java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:3219)
	at
java.awt.LightweightDispatcher.processMouseEvent(Container.java:2930)
	at
java.awt.LightweightDispatcher.dispatchEvent(Container.java:2866)
	at
java.awt.Container.dispatchEventImpl(Container.java:1426)
	at java.awt.Window.dispatchEventImpl(Window.java:1568)
	at java.awt.Component.dispatchEvent(Component.java:3368)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:448)
	at
java.awt.EventDispatchThread.pumpOneEventForHierarchy(EventDispatchThread.java:193)
	at
java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:147)
	at
java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:141)
	at
java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:133)
	at
java.awt.EventDispatchThread.run(EventDispatchThread.java:101)


This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import java.awt.FlowLayout;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import javax.swing.*;
public class TestNullPropChangeNewValuesFromAction extends
AbstractAction {
    public TestNullPropChangeNewValuesFromAction() {
        super("Hi mom");
    }
    public void actionPerformed(ActionEvent e) {
        firePropertyChange("enabled", null, null);
    }
    public static void main (String args[]) {
        JFrame frame = new
JFrame("TestNullPropChangeNewValuesFromAction");
       
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        JMenuBar menubar = new JMenuBar();
        JMenu menu = new JMenu("Test");
        menu.add(new
TestNullPropChangeNewValuesFromAction());
        menubar.add(menu);
        frame.setJMenuBar(menubar);
        frame.getContentPane().setLayout(new FlowLayout());
        JToolBar toolbar = new JToolBar();
        toolbar.add(new
TestNullPropChangeNewValuesFromAction());
        frame.getContentPane().add(toolbar);
        final JButton button = new JButton("Show popup");
        button.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                JPopupMenu popup = new JPopupMenu();
                popup.add(new
TestNullPropChangeNewValuesFromAction());
                popup.show(button, 10, 10);
            }
        });
        frame.getContentPane().add(button);
        frame.pack();
        frame.setVisible(true);
    }
}

---------- END SOURCE ----------

CUSTOMER WORKAROUND :
A work around is to always be sure to pass a newValue (and a
property name). The disadvantage is potential loss of
efficiency, depending on the application.
(Review ID: 137658) 
======================================================================

                                    

Comments
EVALUATION

I've changed all the Action bearing classes to support a null new value.  That is: all of the AbstractButton subclasses, JToolBar, JPopupMenu, JComboBox and JTextField.  As the beans spec says a null value can mean the new value is too expensive to calculate as well as plain old null the new code always calls back to the Action for the value.

As part of this I've conslidated a lot of the similarities and fixed a number of related bugs.  Here's the summary of what's happened:

AbstractButton: The old code encouraged a new ActionPropertyChangeListener per subclass.  This lead to duplicate listeners and configure methods.  In the new code the listener calls back to a method of the button so that it's easier to see what's going on as well as not requiring new listener subclasses for the differences in button types.  An added bonus to this is that all AbstractButton subclasses now have support for mnemonics, which not all previously did via the action.

AbstractActionPropertyChangeListener:  This is now a concrete class.  It handles changes for tool tip and enabeled directly as well as removing itself if the target goes away.  This means JTextField/JComboBox can use this class directly rather than creating a subclass.  It's also been parameterized around the type of class of the target.

JCheckBox: createActionPropertyChangeListener just calls to super.  This change fixs a leak where by the Action could pin down the JCheckBox.  In addition mnemonic changes are now honored.

JComboBox: createActionPropertyChangeListener now just creates an ActionPropertyChangeListener.  This fixs a leak similar to JCheckBox.

JMenuItem: Minor consolidation

JMenu: Removed verbage as to you shouldn't use the various add(Action) methods.  There's no reason not to, they behave the same as setAction.  Listener cleanup so that all types supported via JMenuItem.setAction are supported on Actions added by way of the add variants.  Also fixed bug in that if a JMenu was attached to an action that contained an Accelerator and the Accelerator changed you'd get an exception.

JPopupMenu: Same as JMenu.

JRadioButton: Fixed leak, consolidation.

JTextField: Fixed leak, consolidation.  action command key is now set Action too.

JToolBar: Same as JMenu

I've also fixed serialization bugs such that ActionPropertyChangeListener is now serializable and takes care of everything appropriately.

As is also mentioned in the beans spec a property name of null can indicate everything changed.  I haven't added support for this as I'm a bit worried about folks that are using Actions now and sending out null for something else.  To cause the components to reset themselves in such a case could cause problems.
                                     
2005-08-11
CONVERTED DATA

BugTraq+ Release Management Values

COMMIT TO FIX:
mustang


                                     
2004-09-25
EVALUATION

The Beans Specification is explicit in stating that null values are supported as new and old values then Swing should support this as well. This is similar to the firePropertyChange(null,null,null) bug: 4528354.

Unfortunately, this isn't constrained to Actions since just about all the PropertyChangeListeners in Swing assume that the newValue is not null. The fix for all these PCLs could be risky and provide negligible benefit and could result in a possible performance degradation in Swing since the newValues must allways be checked and there are hundreds of notification events passed around.

We may have to make a judgement call here. Is it not unreasonable for classes which add Swing components as PropertyChangeListeners to provide values instead of null? Even though Swing didn't conform to the letter of the law Beans specification, the PC notification mechanism seemed to have been designed to assume that the new values would have context. 

But then again, SwingPropertyChangeSupport and PropertyChangeSupport will allow newValue == oldValue == null to pass so I assume that this should be supported.

Tough call. I will keep this issue open for discussion.

###@###.### 2002-05-30

After reexamining this issue, we should fix this for the action properties since they are user visible and settable. The PCLs in the LAFs should not bear the same scrutiny since they are usualy listening to the Swing components themselves.

The property change handler in AbstractButton should check for newValue == null. However, I'm not really clear right now what the semantics should be if the newValue == null. Should the null value be passed to the property, igonored, or a reasonable default. What does it mean if both the oldValue == newValue == null?

Will commit and resolve in 1.5.
---
import javax.swing.*;

import java.awt.event.ActionEvent;

// Test for 4626632
public class ATest {

    public static void main(String[] args) {
	Action a = new AbstractAction("test") {
		public void actionPerformed(ActionEvent evt) {}
	    };
	AbstractButton b = new JButton(a);
	a.putValue(Action.MNEMONIC_KEY, null);
    }
}
---

###@###.### 2003-06-25

I think the semantics should be that the newValue be pulled directly from the Action. Having it in the PropertyChangeEvent is merely convenience.
###@###.### 2004-08-24
                                     
2004-08-24



Hardware and Software, Engineered to Work Together