-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix orphans check #2507
Fix orphans check #2507
Conversation
See the example HTML for the effect of preventing line breaks before the first line box: <!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<style type="text/css">
@page {
size: 400pt 300pt;
margin: 50pt;
}
body {
font-family: sans-serif;
letter-spacing: 0.8pt;
border: 1pt solid black;
line-height: 19pt;
}
p {
margin: 0 0 30pt 0;
border: 5pt solid black;
orphans: 1;
widows: 1;
}
</style>
</head>
<body>
<p>Lorem ipsum dolor sit amet, consectetur adipisici elit, sed eiusmod tempor
incidunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis
nostrud exercitation ullamco laboris nisi ut aliquid ex ea commodi consequat.</p>
<p>Quis aute iure reprehenderit in voluptate velit esse cillum dolore eu fugiat
nulla pariatur. Excepteur sint obcaecat cupiditat non proident, sunt in culpa
qui officia deserunt mollit anim id est laborum.</p>
</body>
</html> |
I wonder if we should go ahead and implement the full update to the orphan logic from 1356 (lines 360-380). Just returning a count of the number of lines is inaccurate since it always returns number of rendered lines up to this point instead of the line we're currently checking. This is important considering that the page break logic walks back through previous frames when a page break is disallowed. |
That is true. But I think the thing is (and probably the reason why the original code didn't try to find the exact line number): As long as we are only checking for orphans, checking against the current number of line boxes is actually sufficient. Backtracking within an inline context can only happen due to:
That said, I just realized that the orphans property is checked on the current frame, so it could change while backtracking. But the spec states that If I have not missed anything, then I would hold off implementing the full logic for getting the exact line number until it is actually needed (as it would only incur a slight performance hit right now, without achieving anything). I have updated the PR with a short comment and changed the code to check the |
45cef1b
to
41157d6
Compare
That said, there seem to be cases where a new line box has already been added without backtracking involved. I’ll have to investigate that further. |
Is that a correct example? An LI doesn't represent inline context, unless I'm misunderstanding the example. Via https://www.w3.org/TR/CSS22/generate.html#lists:
I haven't had a chance to think of an example where this could produce incorrect results. I suspect it would be a rare case, but basically the line after the acceptable orphan count would have to result in a non-allowed page break. When Dompdf rewinds to see if there is an acceptable breaking point earlier it would do so within the lines that should not be orphaned since we're only checking the line count and not the current line. Truthfully it's probably a rare case, and anyway it's how things are handled at present. It will become more of an issue when widows are supported. |
Sorry, was a bit unclear there. What I mean is something like the following, when a page break is to occur before the inner <ul>
<li>Inline content
<ul>
…
</ul>
</li>
</ul> The current code will forbid a page break before the inner HTML<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<style>
@page {
size: 400pt 300pt;
margin: 50pt;
}
ul {
counter-reset: li;
}
li {
counter-increment: li;
}
li::before {
content: counter(li);
}
</style>
</head>
<body>
<ul>
<li></li>
<li></li>
<li></li>
<li></li>
<li></li>
<li></li>
<li></li>
<li></li>
<li></li>
<li></li>
<li></li>
<li><ul><li></li></ul></li>
<li></li>
<li></li>
<li></li>
</ul>
</body>
</html> Output: list-bullet.pdf |
#2552 fixes the sample. It will still fail if the inner |
This effectively forbids line breaks before the first line. Also, check the orphans property on the block parent, as it only applies to block container elements per spec. Based on work in dompdf#1356 Fixes dompdf#1617 Co-authored-by: David Sickmiller <david@sickmiller.com>
41157d6
to
033b2c9
Compare
This also effectively forbids line breaks before the first line.
Based on work in #1356
Fixes #1617