Account "float" elements for dynamic-height calculation in Webkit-based browsers

@see https://issues.apache.org/jira/browse/SHINDIG-1155

[johnfargo, 2009-08-18 01:29:20]

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

[mhermanto, 2009-08-18 03:48:59]

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.
On 2009/08/18 01:29:20, johnfargo wrote:
> Could you re-add this comment?

Done.

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);
On 2009/08/18 01:29:20, johnfargo wrote:
> 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);
>   }
> }
> 

Done. I like.

http://codereview.appspot.com/108050/diff/1/2#newcode81
Line 81: * "float" may place a child element outside of the containig parent
element.
On 2009/08/18 01:29:20, johnfargo wrote:
> nit: s/containig/containing

Done.