PDA

View Full Version : [CLOSED] [4.1.3] removeNode bug



zonereseau
10 Nov 2012, 8:22 AM
Hi,

Beginning on line 6952 of ext-all-debug.js of version 4.1.3 there is a problem in the comparaison. You guy added n.tagName.toUpperCase() != 'BODY' without verifying before that n.tagName exists. It currenlty affect me when I try to save my tree checkbox.

2 lines has the problem and should look like this :
if(n && typeof n.tagName == 'string' && n.tagName.toUpperCase() != 'BODY'){

and
if (n && n.parentNode && typeof n.tagName == 'string' && n.tagName.toUpperCase() != 'BODY') {

else when n.tagName is undefined or another type it just jam the script... because undefined.toUpperCase() is not a valid function.

evant
10 Nov 2012, 1:23 PM
In what case is removeNode being passed something that doesn't have a tagName?

zonereseau
10 Nov 2012, 6:05 PM
This happened when I have a TreePanel with columns and a renderer that return back html.


this.treePanel = Ext.create('Ext.tree.Panel',{
title: __('Resources')+' ('+__('Check to give access')+')',
region:'center',
flex:1,
store:treeStore,
rootVisible: false,
useArrows:true,
columns: [{
xtype: 'treecolumn', //this is so we know which column will show the tree
text: __('Resource name'),
flex: 1,
sortable: true,
dataIndex: 'text'
},{
text:__('Allowed'),
dataIndex: 'hasAccess',
width: 60,
sortable: true,
scope:this,
renderer:function(value, metaData, record, rowIndex, colIndex, store, view) {
if(record.data.depth > 1)
return "<div style='padding:2px; text-align:center;'><input type='radio' onclick='Ext.getCmp(\""+this.id+"\").changeHasAccess(\""+record.internalId+"\", this.name);' name='"+this.id+"hasAccess["+record.data.tagName+"]' value='1' "+(value == '1' ? 'checked="checked" ' : '')+"/></div>";
}
......

evant
10 Nov 2012, 11:03 PM
Ok, but how is it removing the node? If you look at the debugger, what is the type of "n" at that time?

Also, assuming this only happens in IE, correct?

zonereseau
11 Nov 2012, 5:24 AM
Hi,

It happen in every browser... I'll try to give you a full example to reproduce the bug... the thing is that currently we are extending a lot of custom class to create a form with a treePanel inside it.

When I do a console.log(n.tagName) my result is an complete DOM element "<input type..." probably because of my renderer function use in my treePanel.

In previous version of ExtJS the comparaison done was if(n.tagName == 'BODY') which wasn't a problem.

evant
12 Nov 2012, 7:58 PM
A test case would be quite useful.

zonereseau
13 Nov 2012, 12:58 PM
Hi,

I just drill down the code to create a test case, this wasn't an easy tasks... I just realized that was linked to one of my field that is called tagName and seems to only be a problem with ExtDirect submit of my form.

I've create a test based in the example folder so it would be easy to you to run.

Create a file... form.html


<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>AMF Grid Example</title>
<link rel="stylesheet" type="text/css" href="../../resources/css/ext-all.css" />
<link rel="stylesheet" type="text/css" href="../shared/example.css" />
<script type="text/javascript" src="../../ext-all-debug.js"></script>
<script type="text/javascript" src="../direct/php/api.php"></script>
<script type="text/javascript" src="form.js"></script>
</head>
<body>
<div id="test"></div>
</body>
</html>


Create a file form.js


Ext.onReady(function() {

Ext.app.REMOTING_API.enableBuffer = 100;
Ext.direct.Manager.addProvider(Ext.app.REMOTING_API);

Ext.create('Ext.container.Viewport', {
layout:'fit',
items:[
this.formPanel = Ext.create('Ext.form.Panel', {

api:{
submit:Profile.updateBasicInfo
},

paramsAsHash: true,

items:[{
xtype:'textfield',
name:'tagName',
fieldLabel:'TagName'
}],

tbar:[{
text:'Save',
scope:this,
handler:function() {
this.formPanel.getForm().submit({
url:'submit.php',
waitTitle:'Please wait',
waitMsg:'Saving...'
});
}
}]
})
]
});
});


When you hit the save button you should received a error message in any browser that tagName has no method 'toUpperCase' and in my case this crash my form.

dongryphon
3 Dec 2012, 9:14 PM
As you surmised, the problem is caused by using a name attribute of "tagName" since that hides the standard DOM element attribute by that name... which we need to check to handle certain special cases. This is why "el.dom.tagName" produces the wrong value - we are expecting this to yield the value of the element's tagName attribute. Sadly, even resorting to getAttribute('tagName') does not help.

After some deliberation we have decided that there is really no good way to work around this class of problem... we could throw in lots of checks to try, but in the end we cannot behave properly when key element attributes get hidden in this way.

There are plenty of other cases where certain values of name attributes could be problematic. To help avoid these issues, we have added a diagnostic message when any of the most likely problematic names are used as the name of a field.