Major form submission improvements.

[Sean Gilligan]

Anyone alive out there?

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

[Sean Gilligan]

I've pulled the changes into the master branch of the main repo:
http://code.google.com/p/iui/source/detail?r=2335ea9d3351d3fba1fe895683776be73f7222e2

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

[Remi Grumeau]

Cool!
I would just ask those modifications:

#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.
>

[Sean Gilligan]

On 3/26/12 12:28 AM, Remi Grumeau wrote:
> #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 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

[Remi Grumeau]

On Tue, Mar 27, 2012 at 4:48 AM, Sean Gilligan <se...@msgilligan.com> wrote:

On 3/26/12 12:28 AM, Remi Grumeau wrote:

#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 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.

for( var i=0, inb=key.length; i<=nb; i++ ) {

  ...

}

is exaclty the same as

var inb=key.length;

for( var i=0; i<=nb; i++ ) {

  ...

}

The optimization is simple: JS engine doesn't have to read the whole array to know its length each time it loops.

I'm agree with this kind of purpose (parse a form variables), array length will hardly be one day more than 50 so it's a small gain. But please JS compilers getting better is not a good reason to avoid script optimizations... (think QMDs or low-end crapdroid smartphones).

 

#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...

Again, no need for the JS engine to deal with two listeners when only one is enough...

[Eric Lindsey]

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. 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. I think the code he's looking for is:

var inb = o.length;
for (var i = 0; i < inb; i++)
{
var value = o[i];

Feel free to substitute nb for inb as the variable name if you want.
Just make sure you do it to both instances of the variable. ;)

[Rémi Grumeau]

Good catch Éric ;)
The i is missing. Correct syntax is:

>>> for( var i=0, inb=key.length; i<=inb; i++ ) {

>>> var value = key[i];

Sent from my iPad

[Sean Gilligan]

Sorry for the delayed response...

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

[Sean Gilligan]

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

[Remi Grumeau]

Well, take the test by yourself: 

http://jsfiddle.net/xhGSH/

Remi

On Tue, Apr 3, 2012 at 11:09 PM, Sean Gilligan <se...@msgilligan.com> wrote:

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.

[Sean Gilligan]

On 4/4/12 1:50 AM, Remi Grumeau wrote:
> Well, take the test by yourself:
> http://jsfiddle.net/xhGSH/

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

[Rémi Grumeau]

On ipad2

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

It matters....

Sent from my iPad

--
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.

[Sean Gilligan]

Try this one on your iPad 2:

http://jsfiddle.net/msgilligan/ExbqR/

-- Sean

[Rémi Grumeau]

[1] Start test

for (var i=0; divs[i]; i++) { }

4 ms

for (var i=0; i<divs.length; i++) { }

1 ms

Sent from my iPad

[M. Sean Gilligan]

Now try this one:

http://jsfiddle.net/msgilligan/ExbqR/

-- Sean

[Remi Grumeau]

Not home now but on blackberry os7, it get

for (var i=0; divs[i]; i++) { }

260 ms

for (var i=0; i<divs.length; i++) { }

246 ms

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.
>

--

Remi

[Remi Grumeau]

For the record, my initial link returns this on blackberry os7

for (var i=0; i<divs.length; i++) { }

60 ms

for (var i=0; divs[i]; i++) { }

1851 ms

for (var i=0, il=divs.length; i<il; i++) { }

40 ms

for (var i in divs) { }

2570 ms
var il=divs.length; while(il--) { }
39 ms

var i=0; while(i<divs.length) { i++; }

60ms
Jquery
8552ms

--

Remi

[Sean Gilligan]

Now reduce the loop count from 100,000 to the maximum number of form elements you ever expect to see in an iUI form.  Then take the resulting times in ms and add them to the round trip time for an Ajax request to the fastest server and express the difference as a percentage.

-- Sean

[Remi Grumeau]

You never know my friend, you never know...