Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Top and Bottom Margin collapse is not as per standards #147

Open
subhadhal opened this issue Nov 10, 2017 · 1 comment
Open

Top and Bottom Margin collapse is not as per standards #147

subhadhal opened this issue Nov 10, 2017 · 1 comment

Comments

@subhadhal
Copy link

Hi @danfickle ,

As per https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Box_Model/Mastering_margin_collapsing :

If there is no border, padding, inline part, block formatting context created, or clearance to separate the margin-top of a block from the margin-top of its first child block, the collapsed margin ends up outside the parent.

In the below scenario, if I provide margin-top to a child block, the margin is not getting applied to the child block. I specifically had to provide border/ padding on the parent block to get the margin.

Here I have provided 0.75in margin on the child block and there is no border/padding or similar property is provided on the parent/ child to offset the margin on the child and apply it. Below is the output that I get, whereas the expectation is to get 0.75in of margin from the top. As per CSS Box Model specifications, the child margin should go beyond your parent block and get applied.

capture

If I open the same html in a browser, it provides 0.75in of margin from the top. Here goes a snapshot of it :

capture1

I analysed the BlockBox class and I feel the collapseTopMargin should take the child blocks parent top margin from its entire DOM tree up till the root. Then we should collapse the top margin of the child block only if the parents margin top exceeds that of the childs. However, this is just a naive approach. There are other subtleties that should be factored in for margin collapsing.

	<head>
		<style type="text/css">
			@page
			{
				size:8.5in 11in;
				-fs-page-orientation: portrait; 
				margin-left: 10px; 
				margin-right:15px;
				margin-top:10px;
				margin-bottom: 0;
			}
		</style>
	</head>
	<body>
		<div style="height:50px;border:solid black;border-width:1px;margin-top:0.75in;">
		</div>
	</body>
</html>
@danfickle
Copy link
Owner

Hi @subhadhal

Firstly, thanks for your detailed write-up and diving into the code. It gave me a big clue as to where to start.

The margin collapsing code seems to be correct. It essentially works as you describe but starts from the top instead of the bottom of the tree. After much debugging, it turns out the bug you found only affects the top boxes on the first page and is caused by this code in BlockBox::layout:

        if (c.isPrint()) {
            PageBox firstPage = c.getRootLayer().getFirstPage(c, this);
            if (firstPage != null && firstPage.getTop() == getAbsY() - getPageClearance()) {
                resetTopMargin(c);
            }
        }

It is run after margin are collapsed and essentially says that the very top element(s) on the first page should have its margin reset to whatever the CSS says, undoing the collapsed margin calculation.

The problem is that I have no idea of the intent of this code, and thus, a little scared of removing them. I guess with few tests available, we can only remove it, and see if anybody complains! What do you think?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants