PDA

View Full Version : [FIXED] [4.0.6] Ext JS unintentionally clobbering global variables/accessing unknown variable



kevinday
15 Oct 2011, 9:30 AM
I'm working on adding an Ext JS front end to an existing very very large application, and we've been tracking down some bugs that were the result of some methods (in our code) that were unintentionally using global variables rather than locally scoped variables, mostly forgetting a "var" on things. Such as:


function foo() {
for (i=0;i<10;i++)
bar();
}
function bar() {
i=20;
}


Both functions were using a global "i" variable, which broke the for loop in foo. To try to make SURE we found all instances of this, I modified Google's Closure Compiler to scan the tree of everything we're importing, and found a few places where Ext JS 4.0.6 might be accessing things globally, or accessing undefined variables.

Basically we're parsing the entire source, generating a tree of the parsed javascript and looking for variables or method calls that are seemingly being accessed without ever being "created" somewhere.

This may well be wrong, I haven't proven any of these, but a quick glance seems to confirm it. This is using the stock ext-all-debug.js from 4.0.6.

ext-4.0.6/ext-all-debug.js:46544 and 46546

if (me.axis) {
axis = chart.axes.get(me.axis);
if (axis) {
out = axis.calcEnds();
minY = out.from || axis.prevMin;
maxY = mmax(out.to || axis.prevMax, 0);
}
}

if (me.yField && !Ext.isNumber(minY)) {
axis = Ext.create('Ext.chart.axis.Axis', {
chart: chart,
fields: [].concat(me.yField)
});
out = axis.calcEnds();
minY = out.from || axis.prevMin;
maxY = mmax(out.to || axis.prevMax, 0);
}

"axis" appears to be a local variable, but is never defined as "var axis" within the getBounds function, so it's clobbering any global use of a variable named "axis".
This is already fixed

ext-4.0.6/ext-all-debug.js:57460

res = String(path).replace(bites, function (all, command, args) {
var vals = [],
isMove = command.toLowerCase() == "m",
res = map[command];
args.replace(val, function (value) {
if (isMove && vals[length] == 2) {
res += vals + map[command == "m" ? "l" : "L"];
vals = [];
}
vals.push(Math.round(value * zoom));
});
return res + gals;

Is that supposed to be vals.length rather than vals[length]?
Agreed, something is messed up here

ext-4.0.6/ext-all-debug.js:65005

getFocusEl: function() {
var me = this,
f = me.focusEl,
defaultComp = me.defaultButton || me.defaultFocus,
t = typeof db,
el,
ct;

db appears to be undefined here, and t doesn't seem to be used anyway.
This method has been refactored, global no longer there

ext-4.0.6/ext-all-debug.js:70326

doLoad : function(start){
if(this.fireEvent('beforechange', this, o) !== false){
this.store.load();
}
},

o is undefined here.
This method is unused and has been removed

ext-4.0.6/ext-all-debug.js:82247

if (selModel.getCurrentPosition) {
pos = selModel.getCurrentPosition();
record = grid.store.getAt(pos.row);
columnHeader = grid.headerCt.getHeaderAtIndex(pos.column);
}

pos needs a var before it.
Seems this is already fixed.

ext-4.0.6/ext-all-debug.js:42803

for (i = 0; i < ln; i++) {
if (excludes[i]) {
continue;
}
value = record.get(fields[i]);
max = mmax(max, value);
min = mmin(min, value);
}


value needs a var before it.
Needs an extra var here, agreed
ext-4.0.6/ext-all-debug.js:17837

onUploadComplete: function(frame, options){
var me = this,

response = {
responseText: '',
responseXML: null
}, doc, firstChild;

try {
doc = frame.contentWindow.document || frame.contentDocument || window.frames[id].document;

id is being defined as a local in upload, so onUploadComplete can't access it.
The code now uses frame.id

ext-4.0.6/ext-all-debug.js:7470

"nth-child" : function(c, a) {
var r = [], ri = -1,
m = nthRe.exec(a == "even" && "2n" || a == "odd" && "2n+1" || !nthRe2.test(a) && "n+" + a || a),
f = (m[1] || 1) - 0, l = m[2] - 0;
for(var i = 0, n; n = c[i]; i++){
var pn = n.parentNode;
if (batch != pn._batch) {

I'm really lost as to what's going on in this area, but it says that batch isn't declared at this point.
Batch does get declared, just in a very roundabout way using an eval.

ext-4.0.6/ext-all-debug.js:6338

{
identity: 'History',
fn: function() {
return !!(window.history && history.pushState);
}
},

I realize this is unimportant, but it looks like history is undeclared here.
Not a huge deal, but I spose it wouldn't hurt to fix.


I'm not 100% sure what's going on here, but could someone have a look and see what you think?

jsakalos
15 Oct 2011, 2:51 PM
I'm moving this thread to Bugs because if there are unintentional global vars in Ext then developers must fix it.

evant
15 Oct 2011, 3:53 PM
I've gone through your post and annotated it with some responses. NB I'm referring to the 4.1 code as a reference.

tvanzoelen
16 Oct 2011, 11:21 PM
Applications also have different behaviour if ui components are added to the viewport from different scopes such as functions. I modified my application in the MVC structure but I still have parts that are added later on to the viewport. It results in a lot of layout bugs. Scrollers not appearing, panels not aligned correctly et cetera. If the entire application is created in a single scope such as the launch function there seems no problem. My question if this bug is related to a case like below?

See. http://www.sencha.com/forum/showthread.php?148111-Grid-columns-not-scrolling-in-Sync.-Scope-Problem

(this is one example of the strange behaviours I noticed, but there are more)