Summary: | margin test for CSS3 calc | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mike Lawther <mikelawther> | ||||||||
Component: | New Bugs | Assignee: | Mike Lawther <mikelawther> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | dbates, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 16662 | ||||||||||
Attachments: |
|
Description
Mike Lawther
2012-01-10 15:48:08 PST
Created attachment 121922 [details]
Patch
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. 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. Created attachment 121978 [details]
Patch
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). Created attachment 122106 [details]
Patch
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. (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">? (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. 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. Comment on attachment 122106 [details] Patch Clearing flags on attachment: 122106 Committed r104783: <http://trac.webkit.org/changeset/104783> All reviewed patches have been landed. Closing bug. |