Bug 76000 – margin test for CSS3 calc

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Mike Lawther]

margin test for CSS3 calc

[Mike Lawther]

Created attachment 121922 [details]
Patch

[Daniel Bates]

Comment on attachment 121922 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=121922&action=review

I suggest writing this test using LayoutTests/fast/js/resources/js-test-{pre, post}.js, which provide convenience functions to assert equality with descriptive output. Using these scripts would also make the output of this test more consistent with the output other PASS/FAIL tests. One example of using these scripts to write a test is: <http://trac.webkit.org/browser/trunk/LayoutTests/fast/css/aspect-ratio-inheritance.html>.

> LayoutTests/css3/calc/margin.html:75
> +        var error = [];
> +        if (width != expectedWidth)
> +            error.push("wrong width");
> +        if (height != expectedHeight)
> +            error.push("wrong height");
> +

It would be beneficial to print the expected width/height and actual width/height in the expected results so as to make it more straight forward to diagnose the reason for test failure should one or more the sub tests fail.

[Mike Lawther]

I agree about printing the actual vs the expected. The problem I had was that cr-linux had a different default margin than WebKit Mac, and since calc() doesn't work yet, the expected values were different. I didn't want to have to maintain two sets of results though. Maybe I'll try setting a margin, and getting calc() to override it instead.

[Mike Lawther]

Created attachment 121978 [details]
Patch

[Mike Lawther]

Hi Daniel - I've changed the test to use the pre/post js libraries.

I've also defaulted the margin to 0px, so it should pass OK on cr-linux (my results were generated on the Mac port).

[Mike Lawther]

Created attachment 122106 [details]
Patch

[Daniel Bates]

Comment on attachment 122106 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122106&action=review

> LayoutTests/css3/calc/margin.html:29
> +        <div><p id="percent-all">This element should have an overall margin of 25 pixels (10% of parent width of 300px minus 5px).</p></div><br/>
> +        <div><p id="percent-left">This element should have a left margin of 25 pixels (10% of parent width of 300px minus 5px).</p></div><br/>
> +        <div><p id="percent-right">This element should have a right margin of 25 pixels (10% of parent width of 300px minus 5px).</p></div><br/>
> +        <div><p id="percent-top">This element should have a top margin of 25 pixels (10% of parent width of 300px minus 5px).</p></div><br/>
> +        <div><p id="percent-bottom">This element should have a bottom margin of 25 pixels (10% of parent width of 300px minus 5px).</p></div><br/>

Are these calculations correct with respect the definitions in <style> above? Notice that the style for the <p> specifies a width and height of 200px and 120px, respectively. Wouldn't a percentage value in the -webkit-calc expression be with respect to the height/width of the <p> (since it explicitly defines such dimensions) instead of its containing block? For example, looking at <p id="percent-left">, I would expect that the value of the margin-left property is -webkit-calc(10% - 5px) = 15px = 10% * 200px - 5px.

[Daniel Bates]

(In reply to comment #7)
> [...] Wouldn't a percentage value in the -webkit-calc expression be with respect to the height/width of the <p> (since it explicitly defines such dimensions) instead of its containing block?

I meant to write:

Wouldn't a percentage value in the -webkit-calc expression be with respect to the height/width of the <p> (since it explicitly defines such dimensions) instead of the height/width of <div id="wrapper">?

[Daniel Bates]

(In reply to comment #8)
> [...]
> Wouldn't a percentage value in the -webkit-calc expression be with respect to the height/width of the <p> (since it explicitly defines such dimensions) instead of the height/width of <div id="wrapper">?

Never mind. I'm not sure why I was thinking this.

[Mike Lawther]

Thanks for the review Daniel! I wrote this test a while ago, and margins were one of the trickiest things to measure, hence the contortions this test goes through.

If you can think of a better way to measure margins, I'll happily update this test.

[WebKit Review Bot]

Comment on attachment 122106 [details]
Patch

Clearing flags on attachment: 122106

Committed r104783: <http://trac.webkit.org/changeset/104783>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.