Skip to main content

Swing actions regression?

8 replies [Last post]
chris_e_brown
Offline
Joined: 2004-09-14

Hi,

I have a Swing application that targets Java 5, and as such can't take advantage of improvements to javax.swing.Action in Mustang. Specifically, the same action requires a different (16x16) icon in the menu bar compared with the toolbar (24x24). Although Mustang adds the LARGE_ICON property to support this, I can't use this in Java 5. I worked around this for Java 5 by creating a "wrapper" action with the bigger icon for the toolbar, wrapping the "real" action added to the menu bar: the workaround makes the wrapper action a PropertyChangeListener for the real action.

However, this stopped working in Mustang (I've been using very recent builds). In Java 5, when the property "enabled" is changed (this is frequent occurrence in my application, based on context), the toolbar icons are enabled/disabled (drawn ghosted) in the toolbar as expected. In Mustang, it doesn't happen.

I looked at this, and it seemed that with both Java 5 and Java 6, calling "Action.setEnabled(true|false)" always correctly fires the property change event, but in Java 6, for the toolbar at least, when I propagate the property change for "enabled", nothing happens.

This is the workaround my "ActionToolBarWrapper" uses:

public void propertyChange(final PropertyChangeEvent evt)
{
if ("enabled".equals(evt.getPropertyName()))
setEnabled(Boolean.TRUE.equals(evt.getNewValue()));
else
putValue(evt.getPropertyName(),evt.getNewValue());
}

...whereas in Java 5, only the last line in the method body was required.

Is this a bug or an implementation detail that's affecting me?

Thanks,
Chris

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
zixle
Offline
Joined: 2004-07-22

Folks,

I completely agree with Jeanette on this one. The core problem is that putValue("enabled", ...) and getValue("enabled") do not necessarily map to that of setEnabled/isEnabled for AbstractAction. I can easily fix this for AbstractAction, but I'm not sure if Chris is using AbstractAction. Chris, are you extending AbstractAction? Have you overriden any of the methods other than actionPerformed?

Thanks,

-Scott

chris_e_brown
Offline
Joined: 2004-09-14

I am indeed extending AbstractAction. Following on from your explanations on the bug report, I can see that in fact logically the pre-1.6 implementations were probably wrong to support the getValue/putValue (the "values" aren't really "properties", they're different in a subtle way, I didn't think it through enough at first).

I probably could change my code to be "correct", but I suspect dropping the implementation detail for correctness would break a lot of code.

Answering your other question, I didn't override anything other than actionPerformed.

- Chris

zixle
Offline
Joined: 2004-07-22

Hi Chris,

Ok, I'll change AbstractAction's putValue("enabled")/getValue("enabled") to access the enable field directly. That way your code will work without change on 1.6 and all will be well.

Thanks!

-Scott

jonmlee
Offline
Joined: 2005-09-06
kleopatra
Offline
Joined: 2003-06-11

> The related JavaDesktop forum thread is at
> http://www.javadesktop.org/forums/thread.jspa?forumID=
> 2&threadID=22165&messageID=136219#136219.

the link is no longer valid (after moving the forum from jdesktop to java.net) Could you please give it in the new coordinates?

> The
> resulting bug is at
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=637
> 1910.

Without seeing the discussion you linked to, the drive of the bug report is a bit unclear to me, so it's hard to comment - will try anyway

first:
there must never be an inconsistency between a newValue as reported by the pCE and the property value returned by querying the source directly (assuming we know the type - which is not necessary). In code:

[code]
public void propertyChange(PropertyChangeEvent evt) {
if ("enabled".equals(evt.getPropertyName())) {
assertEquals(evt.getNewValue(), ((Action) evt.getSource()).isEnabled());
}
}
[/code]

second: (and I think we might disagree on this)
trying to set the enabled property via putValue must do nothing, that is neither change the enabled state nor accept the key/value pair and consequently must not fire a changeEvent

[code]
boolean enabled = action.isEnabled();
action.putValue("enabled", !enabled)
assertEquals(enabled, action.isEnabled());
assertNull(action.getValue("enabled"));
[/code]

So at the core of the bug is the fact that AbstractAction accepts the enabled as a valid key in putValue.

The api doc is vague (read: doesn't say anything) about which keys are allowed in putValue. An educated assumption from the interface signature leads to all keys which are explicitly defined as constants and at the same time would exclude any property which has it's own methods to access. The second half of the assumption is strengthened by the fact that the "enabled" can be regarded as belonging to a different category of properties: it's related to the "command" part of the Action while the values accessible via keys are mere "visuals" (except the new selected but that's a different story http://forums.java.net/jive/thread.jspa?threadID=12734&tstart=0).

As an example to see the difference, take f.i. localization at runtime - doing so will potentially change all properties denoted by the constant keys but not touch the enabled property.

Cheers
Jeanette

loubs001
Offline
Joined: 2006-01-05

I just looked at the Mustang source for AbstractAction.java and if you look at the implementation of setEnabled() you'll see the cause of your problem. Curiously, the enabled property is not implemented in terms of putValue. While every other property is stored in a HashMap like structure, the enabled propery is seperate with it's own private boolean field. That explains why there is a setEnabled method, but no setters for any of the other properties, and that there is no Action.ENABLED property key. This is strange, but I'm sure there is a reason for it. Though I cant think what it possibly could be!

So anyway, your workaround seems to be the correct way to do it. You do need to handle enabled and the other properties seperately. I'd be interested to hear Scott Violet from the Swing team expain why enabled is implemented this way (he blogged about the new changes to Actions a while ago)

chris_e_brown
Offline
Joined: 2004-09-14

No replies here from anyway at Sun (so it would seem), so I'm planning to file a bug report soon as it seems like a regression.

uncle_alice
Offline
Joined: 2003-06-16

You should post this in the "Swing & AWT" forum at JavaDesktop.org:

http://www.javadesktop.org/forums/index.jspa

The Swing Team do read and respond to messages there.