[webkit-reviews] review granted: [Bug 106479] Available height should respect min and max height : [Attachment 181981] Patch

Wed Jan 9 14:22:08 PST 2013

Ojan Vafai <ojan at chromium.org> has granted Robert Hogan <robert at webkit.org>'s
request for review:
Bug 106479: Available height should respect min and max height
https://bugs.webkit.org/show_bug.cgi?id=106479

Attachment 181981: Patch
https://bugs.webkit.org/attachment.cgi?id=181981&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=181981&action=review

> LayoutTests/fast/block/percent-top-respects-max-height.html:23
> +	       .container    {
> +		   background:	  green;
> +		   position: relative;
> +		   height:    600px;
> +		   max-height:	  300px;
> +	       }
> +	       .box    {
> +		   height:    100px;
> +		   position:	relative;
> +		   top:    50%;
> +		   background:	  blue;
> +	       }
> +	       .ref    {
> +		   height:    100px;
> +		   position: absolute;
> +		   top:    150px;
> +		   background:	  red;
> +	       }

Nit: typical style in most webkit tests is to put a single space between the
selector and the curly brace and a single space between the property name and
property value.

> LayoutTests/fast/block/percent-top-respects-max-height.html:32
> +	       <div id="ref" class="ref"></div>
> +	       <div id="box" class="box"></div>

With the changes below, you don't need either of these to have IDs.

> LayoutTests/fast/block/percent-top-respects-max-height.html:35
> +	       document.getElementById("box").setAttribute('data-total-y',
150);

No need to set this in script. You can just set the attribute on the div's
markup directly.

> LayoutTests/fast/block/percent-top-respects-max-height.html:36
> +	       checkLayout('body > div > div')

How about:
checkLayout('.box');

Then you won't get two "PASS" lines when there's only one assert.

> LayoutTests/fast/block/percent-top-respects-min-height.html:1
> +<!DOCTYPE html>

Ditto the above comments.