[DEFER-999] Action gets copied when added to a Container [Archive]

View Full Version : [DEFER-999] Action gets copied when added to a Container

YetAnotherFrank

25 May 2010, 12:04 AM

Ext version tested:

Ext 3.2.1

Adapter used:

ext

css used:

only default ext-all.css

Browser versions tested against:

FF3.6.3 (firebug 1.5.4 installed)

Operating System:

Windows Vista

Description:

An Ext.Action can be added to a toolbar or a menu instead of a button or menu item.
This leads to a button / menu item being created using the given Action.
However, the 'baseAction' property of that button / menu item does not reference the original Action, but a copy of that Action! This is quite annoying when you use a custom Action subclass calling its handler in its own scope, since the 'this' reference changes unexpectedly.

Test Case:

<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title id='title'>Title</title>



<link rel="stylesheet" type="text/css" href="../../resources/css/ext-all.css" />





<script type="text/javascript" src="../../adapter/ext/ext-base.js"></script>
<script type="text/javascript" src="../../ext-all-debug.js"></script>







<script type="text/javascript">
Ext.BLANK_IMAGE_URL = '../../resources/images/default/s.gif';

Ext.onReady(function(){

// Custom action class:
var MyAction = Ext.extend(Ext.Action, {
constructor: function(context) {
this.context = context;
MyAction.superclass.constructor.call(this, {
text: 'MyAction',
handler: this.doIt,
scope: this,
iconCls: 'blist'
});
},
doIt: function(){
Ext.MessageBox.alert('Click',"You clicked on 'Action 1', context: " + this.context);
},
setContext: function(context) {
this.context = context;
}
});
var action = new MyAction('initial data');
var tb = new Ext.Toolbar({
items: [
action // <-- Add the action as a button
],

renderTo: Ext.getBody()
});
var button = tb.items.get(0);
action.setContext("context injected through 'action'");
button.baseAction.setContext("context injected through 'baseAction'");
alert("Is it the same action object? " + (button.baseAction === action ? 'yes' : 'no'));

}); //end onReady
</script>

</head>
<body>
</body>
</html>
Steps to reproduce the problem:

Run sample code.

The result that was expected:

Alert: "Is it the same action object? yes"
When clicking 'MyAction' button: Alert: "You clicked on 'Action 1', context: context injected through 'baseAction'"

The result that occurs instead:

Alert: "Is it the same action object? no"
When clicking 'MyAction' button: Alert: "You clicked on 'Action 1', context: context injected through 'action'"

Debugging already done:

Yes

Possible fix:

Change Ext.Container#createComponent() to not apply config over a new object (the one containing the ownerCt reference). If the intention of your code was to not modify the config object, you could handle Ext.Action config objects ('isAction' property!) as a special case. The following code always modifies the config object and afterwards restores the original state by deleting 'ownerCt'.

Ext.Container.prototype.createComponent = function(config, defaultType){
if (config.render) {
return config;
}
// add in ownerCt at creation time but then immediately
// remove so that onBeforeAdd can handle it
config.ownerCt = this;
var c = Ext.create(config, defaultType || this.defaultType);
delete config.ownerCt;
delete c.initialConfig.ownerCt;
delete c.ownerCt;
return c;
};

YetAnotherFrank

26 May 2010, 11:35 PM

Thanks for setting a status!
Can you explain why you think the change should be deferred? What about changing the "possible fix" to only not-copying the config object if it is an Action? Do you think people rely on the Action object being copied?

Jamie Avins

27 May 2010, 12:24 PM

Any change in behavior like this cannot be put in a patch release. If there is one thing I have learned, if it's even possible to be done one way, people will find a way to rely on it.

YetAnotherFrank

27 May 2010, 11:28 PM

I see. Does that mean that in principle, you agree that the behavior should change, or do you still have to evaluate this?
I think the reason that so few people implement reusable actions may be the problem described above. It is a recurring situation that you want to set / inject some context information when reusing an action. Another thing that could help would be if actions could be reused in a declarative way, similar to component xtypes. For example, it would be nice if you could register your custom Ext.Action subclass in an action manager under some 'atype', and Ext.createComponent() would instantiate the custom action class and hand in the JSON parameters. Like so:

var MyAction = Ext.extend(Ext.Action, {
constructor: function(config) {
this.context = config.context;
MyAction.superclass.constructor.call(this, Ext.apply({
text: 'MyAction',
handler: this.doIt,
scope: this,
iconCls: 'blist'
}, config));
},
...
});
Ext.ActionMgr.registerAction('ux.myAction', MyAction); // hypothetical new API!

new Ext.Toolbar({
items: [
xtype: 'button',
atype: 'ux.myAction',
context: 4711
],
renderTo: Ext.getBody()
});

YetAnotherFrank

12 Mar 2011, 4:32 AM

In the meantime, we found a good workaround.
We patch Ext.Action with the following code, so that a copied action "remembers" its original in initialConfig.baseAction. When its initialConfig is applied over the component, the component thus references the original action, not the copy!

(function(ExtAction) {
var Action = Ext.Action = function Action(config) {
config.baseAction = this; // self-reference, so that button references me, not my copy!
ExtAction.call(this, config);
};
Action.prototype = ExtAction.prototype;
Action.prototype.constructor = Action;
Action.superclass = ExtAction.superclass;
})(Ext.Action);

Most of the code just deals with patching Ext.Action. The crucial line of code is

config.baseAction = this;

If you added this line to the Action constructor in the next Ext 3 maintenance release, do you think it could do any harm for anyone?