Probably worth mentioning, this patch provided by Michael Hermanto. I just uploaded it to the codereview tool :)
On Mon, Aug 17, 2009 at 6:29 PM, <[email protected]> wrote: > Thanks for the patch! A few small suggestions here. > > > http://codereview.appspot.com/108050/diff/1/2 > File > features/src/main/javascript/features/dynamic-height/dynamic-height.js > (left): > > http://codereview.appspot.com/108050/diff/1/2#oldcode64 > Line 64: // scrollHeight already accounts for border-bottom and > padding-bottom. > Could you re-add this comment? > > http://codereview.appspot.com/108050/diff/1/2 > File > features/src/main/javascript/features/dynamic-height/dynamic-height.js > (right): > > http://codereview.appspot.com/108050/diff/1/2#newcode72 > Line 72: result = getHeightForWebkitForSubtree(children[i], result); > 1. could we filter out nodes that are hidden? > 2. rather than recurse, how about using a simple queue? > var q = [ document.body ]; > while (q.length > 0) { > var elem = q.shift(); > for (child in elem.children) { > // maxSoFar calculation > q.push(child); > } > } > > http://codereview.appspot.com/108050/diff/1/2#newcode81 > Line 81: * "float" may place a child element outside of the containig > parent element. > nit: s/containig/containing > > > http://codereview.appspot.com/108050 >
