Code review comment for lp:~openerp-dev/openobject-client-web/one2many-onchange

Seems to fix the bug, so good.

=On the other hand, bugs:=
* Variable `class` generates a syntax error in Safari, and it's a reserved word in javascript anyway so it should not be used.

=Style/coding in JS code:=
* Is not needed as only used once, and `.attr('class') == 'classname'` is bad code, should use jQuery's `.hasClass` method in case e.g. there is more than one class on the element
* Please do not introduce new mochikit code, we're trying to get rid of it, use jQuery instead (e.g. getElement, getFirstParentByTagAndClassName, INPUT)
* Also why all those temporary variables at line 540? They're only used once, their name is not much clearer than they would be within map itself.
* Same with `request`, it's only used once in `addCallback` call, why not just chain `addCallback` with `postJSON`?
* And in any case, `addCallback` is a mochikit API (and so is `postJSON`, transitively), please use jQuery facilities instead in new code.
* Remember to terminate statements with `;` always (line 571), check IDE to see if can not display warning, should be able to.
* jQuery(fld) is called a lot, should probably be memoized to e.g. `$current_field`
* There is code which looks redundant:

    if (getElement('_terp_default_o2m/'+k) && !value) {
        if (getElement('_terp_default_o2m/'+k).value=='' && !value)
            continue;
        else if(getElement('_terp_default_o2m/'+k).value && !value) {
            getElement('_terp_default_o2m/'+k).value = '';
            new ListView(k).reload();
        }
    }

  First line already mandates `!value` test, why re-test it at both other cases? And if you remove it, tests at line 2 and line 4 are pretty much opposite of each other so should be able to do with only one test (and an `else` clause rather than `else if`) no?

= In listgrid.py =
* Why is simplejson imported within method (and therefore re-imported at each execution) rather than just at module toplevel?
* Why do you need copy.deepcopy?
* Please avoid using eval unless you have to. What is content of o2m_view_id? Why can't you use `literal_eval` or an explicit call constructor call?
* Same question for o2m_context and o2m_values (which is first loaded via simplejson and then eval'ed?)
* line 39, doesn't make sense to use `.get`:

    view = view['fields'].get(name).get('views').get(o2m_view_type)

  if `name` is not in `view['fields']`. `.get(name)` is going to return `None` which is going to raise an error when trying to call `.get('views')` on it. So why use `.get`, why not just `[name]`? Same question for the calls after that.