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

Reply via email to