PDA

View Full Version : [FIXED][3.0] Ext.urlEncode bug



mschwartz
15 Apr 2009, 12:29 PM
My code does this:



grid.store.load({ params: { start: 0, limit: 25 }});


On the server, $_REQUEST['start'] is ''

So I step through ext-all-debug.js and at line 5851:


5851 p = Ext.urlEncode(me.extraParams, typeof p == 'object' ? Ext.urlEncode(p) : p);


Examine p after and it's this:
start=&limit=25&xaction=load

start= should be start=0

mschwartz
15 Apr 2009, 12:35 PM
I don't have the source to Ext.urlEncode in the rc1.zip file

mschwartz
15 Apr 2009, 12:52 PM
Downloaded the source.

The bug is here:

Ext.js, lines 294-298:



for(key in o) {
Ext.each(o[key] || key, function(val, i) {
buf.push("&", e(key), "=", val != key ? e(val) : "");
});
}

christocracy
15 Apr 2009, 12:55 PM
Do you get same behaviour if start > 0?

evant
15 Apr 2009, 12:57 PM
The code in 3.x looks very different to the code in 2.x. We'll look into this.

mschwartz
15 Apr 2009, 12:57 PM
No.

start: 1

works

mschwartz
15 Apr 2009, 12:58 PM
The code in 3.x looks very different to the code in 2.x. We'll look into this.

Not really sure what you're trying to do there with comparing the key and value...

mschwartz
15 Apr 2009, 12:59 PM
start: '0' works

mpf
15 Apr 2009, 11:28 PM
I'm seeing this issue as well in any widget that use a PagingToolbar. I also tracked it down to Ext.urlEncode().

>>> Ext.urlEncode({'foo':0})
"foo="

Mitz
16 Apr 2009, 5:51 AM
I encountered this nasty bug as well.. it seems like all GET parameters with integer value 0 have their value stripped.

To reproduce:

Ext.Ajax.request({ url: '/blank', params: { foo: 0 }, method: 'GET' })

The value of foo will be stripped in the resulting URL.

evant
16 Apr 2009, 5:52 AM
Yep, the bug is confirmed, we're looking into it.

mschwartz
16 Apr 2009, 5:54 AM
This fix seems to work for me.



Ext.apply(Ext, {
urlEncode: function(o, pre) {
var buf = [],
key,
e = encodeURIComponent;

for (key in o) {
Ext.each((o[key] !== undefined) ? o[key] : key,
function(val, i) {
buf.push("&", e(key), "=", val != key ? e(val) : "");
});
}
if (!pre) {
buf.shift();
pre = "";
}
return pre + buf.join('');
}
});
And the leading zero in the paging grid is gone with this fix, too :)

The 2.2 urlEncode severely broke everything, FWIW

mschwartz
16 Apr 2009, 6:06 AM
Similar report here:

http://extjs.com/forum/showthread.php?t=65706

(so you guys can keep track)

jacombs
16 Apr 2009, 6:35 AM
I hit this bug this morning.

Mitz
16 Apr 2009, 6:51 AM
Thanks for the fix, mschwartz. Works beautifully here. Here's hoping that it gets fixed in the next RC as well.. time for better regression testing perhaps? ;-)

mschwartz
16 Apr 2009, 6:53 AM
Thanks for the fix, mschwartz. Works beautifully here. Here's hoping that it gets fixed in the next RC as well.. time for better regression testing perhaps? ;-)

Our code is the regression test!
:D

hendricd
22 Apr 2009, 6:06 PM
In 3.0

Ext.urlEncode({a:null , b:3 }) yields a=&b=3

Ext 2.2 and earlier yielded b=3

Is the null value behavior by design or a boo-boo? ;)

mschwartz
23 Apr 2009, 4:06 AM
In 3.0

Ext.urlEncode({a:null , b:3 }) yields a=&b=3

Ext 2.2 and earlier yielded b=3

Is the null value behavior by design or a boo-boo? ;)

The former is the proper behavior, right?

hendricd
23 Apr 2009, 4:12 AM
The former is the proper behavior, right?

Per the HTTP RFC, that is valid, but the change suddenly broke much (my) code designed for <= 2.x Apps.

If that (former) is the new behavior, we need to let everyone know in the release notes.

mschwartz
23 Apr 2009, 4:28 AM
Per the HTTP RFC, that is valid, but the change suddenly broke much (my) code designed for <= 2.x Apps.

If that (former) is the new behavior, we need to let everyone know in the release notes.

I agree a change in behavior is going to be a monkey wrench for some things, but an X.0 type version change/release is a really good time to get things right.

Enough's already changed that I've spent quite a bit of time debugging and patching up several of the Ext.ux type extensions generously provided by folks here, as well as a few in the examples directory (we're not alone!). It's well worth it, so things like miframe (awesome!) don't get deprecated or depreciated with the version change.

The HTTP spec says checkboxes return 'on' if set and nothing if not, but Ext doesn't have to support that - checkboxes can return true/false always and they'd be fixing this botched thing in the RFC and making using Ext more friendly at the same time. Without breaking any rules.

hendricd
23 Apr 2009, 7:06 AM
@mschwartz -- I saw somewhere, you had a patch for miframe (for 3.0) what/where is it?
I'll have to make sure it gets considered for MIF 2.0. ;)

mschwartz
23 Apr 2009, 10:22 AM
@mschwartz -- I saw somewhere, you had a patch for miframe (for 3.0) what/where is it?
I'll have to make sure it gets considered for MIF 2.0. ;)

https://extjs.com/forum/showthread.php?p=316909#post316909

It maybe should be MIF 3.0

Or 3.2 or 2.3 (if you get my meaning)

galdaka
26 Apr 2009, 11:51 PM
Bug still not solved in SVN (Revision 3838).

The functionallity of grid of most applications depend of this error. ;)

Greetings,

nctag
27 Apr 2009, 5:19 AM
Exactly what I just wanted to say.

The bug is still not solved in Revision 3838. The speculations wheter it is a change in logic or it is a bug are really confusing. If there is no official statement about a logic change I mean it must be a bug.

I ask myself why it's not been solved yet? Regularely major bugs are fixed ASAP.

John Sourcer
27 Apr 2009, 11:55 PM
Shagged me as well.

Just to clarify:




var MY_VAR = 0;

ds.baseParams = {my_var_test_id: MY_VAR, ....



Posts my_var_test_id as ''

mschwartz
28 Apr 2009, 4:45 AM
See post #12 in this thread.

nctag
28 Apr 2009, 6:11 AM
off course it's a nice workaround. Thanks for that. Because of it I hadn't have to debug it myself, but we're all waiting for an official fix in SVN or a statement here... aren't we?!

mschwartz
28 Apr 2009, 6:19 AM
off course it's a nice workaround. Thanks for that. Because of it I hadn't have to debug it myself, but we're all waiting for an official fix in SVN or a statement here... aren't we?!

We are.

What I've been doing is maintaining a file called Ext.overrides.js with various snippets like the one in post #12. When the next release of Ext 3 comes out, I can comment out the snippets and see if things work (e.g. are fixed).

My strategy is to work with the release candidate and 3.x right now so my project won't be left in the dust when 2.x becomes obsolete (for lack of a better word).

nctag
28 Apr 2009, 6:37 AM
The system with the overrides.js is a must have when you are working with Ext and want to be flexible and fast as well. I also work as you said. (As hint for other users: include the link to the threads discussing about the bugs into the comments of the methods so you are easily able to check whether there are some updates or not.)

I am currently blackbox testing the whole application including the extensions. Am I right that there is no migration guide? (from Ext 1.0 to Ext 2.0 I think there was one)

evant
28 Apr 2009, 7:15 AM
Try the following:



Ext.apply(Ext, {
each: function(array, fn, scope){
if(Ext.isEmpty(array, true))
return;
if(typeof array.length == "undefined" || Ext.isPrimitive(array)){
array = [array];
}
for(var i = 0, len = array.length; i < len; i++){
if(fn.call(scope || array[i], array[i], i, array) === false){
return i;
};
}
},

urlEncode: function(o, pre){
var buf = [], key, item, e = encodeURIComponent;

for(key in o){
item = o[key];
Ext.each(typeof item == 'undefined' ? key : item, function(val, i){
buf.push("&", e(key), "=", val != key ? e(val) : "");
});
}
if(!pre){
buf.shift();
pre = "";
}
return pre + buf.join('');
}
});

galdaka
28 Apr 2009, 10:28 AM
Works fine.

Will you add this override to a SVN?

Thanks and greetings,

CCZu
29 Apr 2009, 7:14 AM
Thanks a lot

nctag
29 Apr 2009, 2:06 PM
@evant
Thanks, but we've found a workaround earlier and mschwartz has it already posted. But will there be a fix in SVN soon?

evant
29 Apr 2009, 5:38 PM
It's in SVN.

nctag
29 Apr 2009, 10:33 PM
All right right, it's all my fault.
Evant, thank you very much for your effort. Great job.

galdaka
5 May 2009, 12:20 AM
Hi,

The problem still present in ExtJS 3.0 rc1.1.

Greetings,

evant
5 May 2009, 1:29 AM
Seems like the latest ext-core wasn't included with this build. It is still fixed however.

Laurent Mignon
7 Jul 2009, 8:08 AM
Try the following:



Ext.apply(Ext, {
each: function(array, fn, scope){
if(Ext.isEmpty(array, true))
return;
if(typeof array.length == "undefined" || Ext.isPrimitive(array)){
array = [array];
}
for(var i = 0, len = array.length; i < len; i++){
if(fn.call(scope || array[i], array[i], i, array) === false){
return i;
};
}
},

urlEncode: function(o, pre){
var buf = [], key, item, e = encodeURIComponent;

for(key in o){
item = o[key];
Ext.each(typeof item == 'undefined' ? key : item, function(val, i){
buf.push("&", e(key), "=", val != key ? e(val) : "");
});
}
if(!pre){
buf.shift();
pre = "";
}
return pre + buf.join('');
}
});

In the RC3 I issue the same kind of problem when I initialize baseParams with an empty array {my-param-list: [ ]}. The my-param-list never appears in the posted form when my store load.
I've fixed this by modifying the previous code as follow...


Ext.apply(Ext, {
each: function(array, fn, scope){
if(Ext.isEmpty(array, true))
return;
if(typeof array.length == "undefined" || Ext.isPrimitive(array)){
array = [array];
}
for(var i = 0, len = array.length; i < len; i++){
if(fn.call(scope || array[i], array[i], i, array) === false){
return i;
};
}
},

urlEncode: function(o, pre){
var buf = [], key, item, e = encodeURIComponent;

for(key in o){
item = o[key];
Ext.each(typeof item == 'undefined' || Ext.isEmpty(item, true)? key : item, function(val, i){
buf.push("&", e(key), "=", val != key ? e(val) : "");
});
}
if(!pre){
buf.shift();
pre = "";
}
return pre + buf.join('');
}
});

mjlecomte
7 Jul 2009, 10:25 AM
@Laurent

Thanks for your report. I would suggest that you post in a new thread that is not marked FIXED. If it is related to this thread, feel free to provide a link back to this thread in your report that conforms with this thread:

http://extjs.com/forum/showthread.php?t=71015

Make sure to post a test case that illustrates the undesirable behavior.