PDA

View Full Version : Wrong usage of groupId in GroupingView



steffenk
1 Mar 2011, 10:44 AM
the groupID results from the field value of the record if you group by a record field.
This is dangerous, as the generated id may contain invalid characters.

The template is build like this:

this.startGroup = new Ext.XTemplate(
'<div id="{groupId}" class="x-grid-group {cls}">',
'<div id="{groupId}-hd" class="x-grid-group-hd" style="{style}"><div class="x-grid-group-title">', this.groupTextTpl ,'</div></div>',
'<div id="{groupId}-bd" class="x-grid-group-body">'
);

At least there should be an encoding for the id or replace of invalid characters. Wrong id can cause problems in weak browsers like IE.

Condor
2 Mar 2011, 8:28 AM
Ext.grid.GroupingView.prototype.constructId already htmlEncodes the groupId.

steffenk
2 Mar 2011, 9:44 AM
this is an example of a rendered id:
id="ext-gen13-gp-path_Workspace-/Home/-bd"

i can't belive that this is ok having slashes in.

I suggest a replace with regular expression having the valid chars instead of htmlEncode.

Condor
3 Mar 2011, 12:31 AM
Most browsers don't care as long as you don't include a " or a < in the attribute (which htmlEncode correctly escapes).

A better solution would require a rewrite of GroupingView, because you would need to replace the group value with a generated id and use a internal lookup hash.

steffenk
3 Mar 2011, 3:39 AM
unfortunally IE is not lucky with such id and generates an error.
I'll look to make an easy override first, but i agree that a generated id would be the better solution.

steffenk
3 Mar 2011, 4:46 AM
i now use this override which does the job


Ext.override(Ext.grid.GroupingView, {
constructId : function(value, field, idx){
var cfg = this.cm.config[idx],
groupRenderer = cfg.groupRenderer || cfg.renderer,
val = (this.groupMode == 'value') ? value : this.getGroup(value, {data:{}}, groupRenderer, 0, idx, this.ds);

var id = this.getPrefix(field) + val;
id = id.replace(/[^a-zA-Z0-9_]/g, '');
return id;
}
});

Condor
3 Mar 2011, 11:38 PM
No, that's not a proper solution. It could create duplicate id's (e.g. group '123-456' and '12-34-56' would end up with the same id).

steffenk
3 Mar 2011, 11:54 PM
I know, in my case there are paths where it can't happen.

Anyway, double id can also happen with the existing way if there are duplicate values.
Proper solution would be an internal storage with generated id's and a lookup scheme as you already suggested.

Sesshomurai
11 Mar 2011, 6:42 AM
Why not just Base64 the key and use that internally?