PDA

View Full Version : [DEFER-475] initRef fails with too many levels



VinylFox
1 Jan 2010, 9:57 AM
I need some feedback from the team about making this change / if it should be made.

Ext version tested:

Ext 3.1 rev 5766


Adapter used:

ext


Description:

If too many levels are specified for the ref config, it will find undefined. This is a change in logic from previous versions, not necessarily a bug. In fact, from a technical standpoint it works more accurately now than it did before...however its still different from previous versions.


Test Case:
Run this code on any of the example pages. Since rev 5766 this code will fail. Previous versions will work.

new Ext.Window({
title: 'Test',
id: 'test',
width: 200,
height: 100,
html: 'Bwah bwah',
bbar: [{
text: 'Boo',
ref: '../../../../../buttonBoo',
handler: function(){
Ext.getCmp('test').buttonBoo.setText('New Boo');
Ext.getCmp('test').buttonBleh.setText('New Bleh').disable();
}
}, {
text: 'Bleh',
ref: '../../../../../buttonBleh'
}]
}).show();

This is because an if statement was removed in the loop that checks owners, allowing undefined to be set as the last item found in the owner chain.

Fix:

Ext.override(Ext.Component, {
initRef : function() {
if(this.ref && !this.refOwner){
var levels = this.ref.split('/'),
last = levels.length,
i = 0,
t = this;

while(t && i < last){
if (t.ownerCt) {
t = t.ownerCt;
}
++i;
}
if(t){
t[this.refName = levels[--i]] = this;
this.refOwner = t;
}
}
}
});

This fix promotes sloppy code, allowing users to specify more levels than exist in a ref, however this is how it has worked previously.

Animal
1 Jan 2010, 10:40 AM
Not necessarily sloppy. They could just mean "The outermost Container" by using "/////////" and relying on it to stop at the top.

Of course if the ref really was some kind of parsed specifier instead of just a string in which "/" characters are counted, they could maybe use



ref: '#viewport'


http://www.extjs.com/forum/showthread.php?p=380000#post380000

VinylFox
8 Jan 2010, 8:36 AM
Fixed in rev 5862 with suggested change.

I love that concept Nige. Will discuss more in the thread for it.

Animal
8 Jan 2010, 8:37 AM
Yes. ComponentQuery just like DomQuery. I think there's a good case for it.

evant
24 Jan 2010, 4:18 PM
The change in behaviour for this was on purpose, it actually causes bugs in some situations. Here's a fairly simple counter test case:



Ext.onReady(function(){
//var refPanel =

var vp = new Ext.Viewport({
layout: 'border',
id: 'vp',
items: [
new Ext.Panel({
region: 'center',
xtype: 'panel',
ref: 'refPanel1',
title: 'Ref Panel 1',
bodyStyle: 'padding:10px',
items: [{
xtype: 'panel',
ref: '../refPanel2',
title: 'Ref Panel 2',
frame: true,
html: 'this is ref panel 2'
}]
})
]
})
});


I think there's merit in the ideas regarding some kind of simple query, though I'm not that keen on it for 2 reasons:

1) Will it be too complicated, both in terms of execution and usage to be useful.
2) Using the example of '#viewport', when do we actually execute this? In the above code, the outermost component will be 'Ref Panel 1' until it's added to the viewport.

Anyway, food for thought, I'd be interested to hear what others think.

VinylFox
26 Jan 2010, 3:01 PM
The change in behaviour for this was on purpose, it actually causes bugs in some situations...

Wish anyone from the Ext JS team had said that a month ago when I posted this bug...or a week later when I decided to fix it.

It's disappointing when bug reports are ignored. A little communication can go a long way.

Jamie Avins
27 Jan 2010, 6:48 AM
Hiring a new QA person to get things into Trac is a high priority. If you have any issues not given a proper bug number, please feel free to bump them so I can make sure they get addressed properly.

Jamie Avins
18 Feb 2010, 5:47 PM
Re-opening this issue for a possible enhancement to 3.2.0.