I combined 4 Ajax form submission bugs and fixed them all today.
The are:
#48) Checkboxes not submitted (now renamed to include all 4 issues)
http://code.google.com/p/iui/issues/detail?id=48
#67) Disabled input field are submitted
http://code.google.com/p/iui/issues/detail?id=67
#72) Multiple submit buttons don't work:
http://code.google.com/p/iui/issues/detail?id=72
#77) radio buttons get submitted incorrectly
http://code.google.com/p/iui/issues/detail?id=77
The fix -- WHICH I WOULD LIKE PEOPLE TO REVIEW -- is here:
http://code.google.com/r/msgilligan-iui-dev/source/detail?r=2335ea9d3351d3fba1fe895683776be73f7222e2&name=msgilligan-issue72
I also created some tests and published them here:
http://stage.iui-js.org/test/form-test.html#_arrays (to test Radios and
Checkboxes)
and here:
http://stage.iui-js.org/test/form-test.html#_submits (to test multiple
submit buttons and disabled inputs)
These ware egregious and longstanding bugs. I'd really like people to
review the fixes and, if it looks good, I'll put it in an 0.40-beta3.
-- Sean
Thanks, Eric for the +1!
Would still love code reviews or testing on the fix. It's not too late
to help out.
Thanks,
Sean
#1 : I would replace
for ( var key in o) {
var value = o[key];
by
for( var i=0, inb=key.length; i<=nb; i++ ) {
var value = key[i];
regarding some JS loop tests i've done
http://www.we-are-gurus.com/blog/1884-optimizing-javascript-loops-for-execution-speed
Same of course for the loop inside it if value is "object".
#2 I never understood why we had two addEventListener on the click event... now we have 3 !??!
Why not just put everything in the same listener ?
Remi
> --
> You received this message because you are subscribed to the Google Groups "iui-developers" group.
> To post to this group, send email to iui-dev...@googlegroups.com.
> To unsubscribe from this group, send email to iui-developer...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/iui-developers?hl=en.
>
I don't think the replacement code works.
I prefer not to do loop optimization like that and just keep the code
simple. The JavaScript compilers are getting better and better and
before long those types of optimizations will make little difference.
In the meantime we should keep the code as simple as possible for ease
of reading and to prevent mistakes.
> #2 I never understood why we had two addEventListener on the click event... now we have 3 !??!
> Why not just put everything in the same listener ?
I think the idea of having separate listeners is that they are more
modular and can be turned on/off individually. Joe created the first
two and I (following Joe's style) created the third.
That said, I do think they can (should) be combined. I'll do that soon...
-- Sean
On 3/26/12 12:28 AM, Remi Grumeau wrote:I don't think the replacement code works.
#1 : I would replace
for ( var key in o) {
var value = o[key];
by
for( var i=0, inb=key.length; i<=nb; i++ ) {
var value = key[i];
I prefer not to do loop optimization like that and just keep the code simple. The JavaScript compilers are getting better and better and before long those types of optimizations will make little difference. In the meantime we should keep the code as simple as possible for ease of reading and to prevent mistakes.
I think the idea of having separate listeners is that they are more modular and can be turned on/off individually. Joe created the first two and I (following Joe's style) created the third.
#2 I never understood why we had two addEventListener on the click event... now we have 3 !??!
Why not just put everything in the same listener ?
That said, I do think they can (should) be combined. I'll do that soon...
>>> for( var i=0, inb=key.length; i<=inb; i++ ) {
>>> var value = key[i];
Sent from my iPad
On 3/27/12 9:38 PM, Eric Lindsey wrote:
> The replacement code doesn't work because Remi made a typo with the
> variable names. inb != nb. Anyways, I agree with him that we should
> use smart coding practices and apply easy optimizations just as you
> would in normal C code.
I programmed in 'C' for many, many years. I didn't make those
optimizations there, either. I strongly feel we should not make these
types of optimizations to the source code unless there is a demonstrable
performance gain.
> The for...in uses an iterator and
> accommodates field names. In this case, it is faster and more
> efficient to use the plain old for loop with the integer array
> indexer.
In this particular case, 'o' is an Object, not an array. So this
optimization won't work.
-- Sean
Still won't work -- see my response to Eric.
-- Sean
On 3/28/12 10:13 AM, Rémi Grumeau wrote:
Good catch Éric ;)
The i is missing. Correct syntax is:
for( var i=0, inb=key.length; i<=inb; i++ ) {
var value = key[i];
Still won't work -- see my response to Eric.
-- Sean
--
You received this message because you are subscribed to the Google Groups "iui-developers" group.
To post to this group, send email to iui-developers@googlegroups.com.
To unsubscribe from this group, send email to iui-developers+unsubscribe@googlegroups.com.
Nice.
I forked it and moved the HTML, CSS, and JS into the separate panes to
make it easier to hack on:
http://jsfiddle.net/msgilligan/Cqtf3/
-- Sean
for (var i=0; i<divs.length; i++) { } | 73 ms |
for (var i=0; divs[i]; i++) { } | 169 ms |
for (var i=0, il=divs.length; i<il; i++) { } | 55 ms |
for (var i in divs) { } | 300 ms |
var il=divs.length; while(il--) { } | 55 ms |
var i=0; while(i<divs.length) { i++; } | 71 ms |
JQuery - $('div') - 373ms |
--
You received this message because you are subscribed to the Google Groups "iui-developers" group.
To post to this group, send email to iui-dev...@googlegroups.com.
To unsubscribe from this group, send email to iui-developer...@googlegroups.com.
http://jsfiddle.net/msgilligan/ExbqR/
-- Sean
for (var i=0; divs[i]; i++) { } |
4 ms |
for (var i=0; i<divs.length; i++) { } |
1 ms |