Name: yyT116575			Date: 05/11/2001


The bugs described are platform-independent.

java version "1.3.0"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.3.0-C)
Java HotSpot(TM) Client VM (build 1.3.0-C, mixed mode)


This bug report tries to identify the root cause of several bugs: 4281558,
4381361, 4347339, 4304129, 4391622, 4284401, 4396953, 4409290, 4106486, 4133256,
4304129, 4138015, 4284962, 4266524.
Some of these have been closed as duplicates. Some of these have been fixed.
Some of these have been marked "fixed" even though problems remain. The point
I'm trying to make is this: These problems are vexing enough that more than a
dozen people went to the trouble of submitting a bug report. Even though
work-arounds exist, these problems deserve to be fixed properly.

Here's what I've concluded about the cause of these problems.

Primary cause: The subclasses of AbstractButton are not taking advantage of
inheritance when handling Actions. When they override the
configurePropertiesFromAction() method, they take it upon themselves to
incorporate all the properties specified in the Action. While this is ok in
theory, it has resulted in subclasses remaining broken after the superclass has
been fixed, and other similar problems.

Secondary cause: The implementation of configurePropertiesFromAction() in class
AbstractButton has some simple flaws. These should be easy to fix.
  (flaw1) It doesn't honor ACTION_COMMAND_KEY.
  (flaw2) It doesn't honor ACCELERATOR_KEY.
  (flaw3) If no MNEMONIC_KEY is specified in the Action, it leaves the previous
mnemonic in effect instead of removing it.


  Fixing the Primary cause is less easy, but doable. For example, JCheckBox and
JRadioButton need to override configurePropertiesFromAction() because they can't
allow the SMALL_ICON specified in an Action to clobber their internal 'icon' and
'selectedIcon' properties (which by default are handled by the LnF).
But it shouldn't have to mess with NAME, ACTION_COMMAND_KEY, SHORT_DESCRIPTION,
MNEMONIC_KEY, or ACCELERATOR_KEY--that's what superclasses are for.
Their configurePropertiesFromAction() method should look something like this:

protected void configurePropertiesFromAction(Action a) {
    // define inner class
    class ActionWrapper implements Action {
        private Action w;
        public ActionWrapper(Action w) { this.w = w; }
        public Object getValue(String k) { return w.getValue(k); }
        public void putValue(String k, Object v) { w.putValue(k, v); }
        public boolean isEnabled() { return w.isEnabled(); }
        public void setEnabled(boolean b) { w.setEnabled(b); }
        public void addPropertyChangeListener(PropertyChangeListener l) {
            w.addPropertyChangeListener(l); }
        public void removePropertyChangeListener(PropertyChangeListener l) {
            w.removePropertyChangeListener(l); }
        public void actionPerformed(ActionEvent e) { w.actionPerformed(e); }
    }
    // create a new Action just like 'a' except for its Icon
    final Icon myIcon = getIcon();
    Action myIconAction = new ActionWrapper(a) {
        public Object getValue(String k) {
            if (Action.SMALL_ICON.equals(k)) return myIcon;
            return super.getValue(k);
        }
    };
    // invoke parent's method on the altered Action
    super.configurePropertiesFromAction(myIconAction);
}

Similarly, if JMenuItem is documented to not support toolTipText, it should do
the same kind of thing to mask out SHORT_DESCRIPTION without messing with the
other five properties.
Also, the button created by JToolBar.add(Action a) in
createActionComponent(Action a) should do the same kind of thing to mask out
NAME. (See bug 4106486, which is marked as fixed in swing1.0fcs even though it
is still broken in 1.3.)

Note that, while I do suggest this general approach, I'm not suggesting that
this code be used verbatim. At the very least, the ActionWrapper class should be
moved outside the body of the method, preferably to someplace where it can be
used by programmers who wish to subclass the existing button types.


I didn't provide any example programs that exhibit the bugs that need to be
fixed. Frankly, there are so many individual bugs with the way the various
button classes handle Actions that it would be unwieldy to demonstrate--or even
list--them all. (seven button classes, each with a setAction() method, an Action
constructor, and Action-related PropertyChangeListener code)
On request I can supply several small programs that exhibit one or two bugs
each. And there are several example programs in the other bug reports I
mentioned.


One final note: Anyone planning to fix this should also take a look at
AbstractButton.ButtonActionPropertyChangeListener and any other subclasses of
AbstractActionPropertyChangeListener (anonymous or otherwise) that subclasses of
AbstractButton create. These things maybe need the same kind of help.
(Review ID: 124245) 
======================================================================