[webkit-reviews] review denied: [Bug 45944] Update percent-image-height layout test to include additional parent element positioning modes. : [Attachment 67880] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied Luke Macpherson
<macpherson at chromium.org>'s request for review:
Bug 45944: Update percent-image-height layout test to include additional parent
element positioning modes.
https://bugs.webkit.org/show_bug.cgi?id=45944

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

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

> LayoutTests/fast/images/percent-height-image.html:34
> +    function runTests() {

Here and below: WebKit style is to put the opening bracket on the next line.
Not all the JS code follows the style since there's no formal WebKit JS style,
but we're trying to push it in that direction to make it match C++.

> LayoutTests/fast/images/percent-height-image.html:43
> +	   document.querySelector("body").className = name;

how about:
document.getElementById('parent').className = name.

Then  you can change the selectors in the CSS above to just have the className.
I think that would make the test a bit more readable.

> LayoutTests/fast/images/percent-height-image.html:48
> +	       log(className + " FAIL: image height was " + actual + " expected
" + expected);

Indentation is off.

> LayoutTests/fast/images/percent-height-image.html:56
> +    Tests that an image with height 100% inside a floated div keeps its
intrinsic height.<br/>

This description is no longer accurate.