Code review comment for lp:~serpentcs/openerp-web/fix-1191037-web

Revision history for this message
Xavier (Open ERP) (xmo-deactivatedaccount) wrote :

- tabs
- screwed indentation
- leftover condition on `value` in moved if is useless
- caller expects a string, not `undefined`. The error is not seen here because the caller simply passes the value to $.fn.html and does not do anything with the result of $.fn.html. Passing undefined makes $.fn.html return the html content instead of setting it, if more operations are chained on the node in the future, the rendering will blow up on empty binary fields.
- 45 lines of diff where a single one would have sufficed, why not just

    if (!value) { return ''; }

  added right before the existing conditionals?

review: Disapprove

« Back to merge proposal