"break-after: page" doesn't reliably push the next sibling to next page; it might get a continuation on same page if it has a zero-height first child, which technically "fits" after a nsPageBreakFrame)
Categories
(Core :: Layout: Block and Inline, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox118 | --- | fixed |
People
(Reporter: dholbert, Assigned: alaskanemily)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
[spinning this out from a discussion with AlaskanEmily just now]
We implement break-after:page with nsPageBreakFrame which works by eating up all the remaining available-BSize on a page.
If the next thing is a zero-height frame like a nsPlaceholderFrame (or a block that contains a zero-height thing at its very top), then we typically won't put that zero-height frame on the next page like we should, because it seems to fit in the zero remaining space on the first page.
We should probably add some sort of special-case to handle this situation by forcing a fragmentation break after we reflow a line that has a nsPageBreakFrame. I think this would help with the Google Docs issue in bug 1816345 (at least with the first testcase there).
|
Reporter | |
Updated•2 years ago
|
|
|
Reporter | |
Comment 1•2 years ago
|
||
Here's one testcase.
STR: Print-preview this testcase.
ACTUAL RESULTS: One page of output.
EXPECTED RESULTS: Two pages of output.
Firefox Nightly gives ACTUAL RESULTS right now. (We place the empty unstyled div at the bottom of page 1 (after the nsPlaceholderFrame) and we find that it fits.) Per this bug, we probably shouldn't even attempt to do that.
Chrome gives EXPECTED RESULTS.
|
|
Reporter | |
Comment 2•2 years ago
|
||
This testcase is a bit closer to the Google Docs scenario.
STR: Print preview this testcase
ACTUAL RESULTS: Page 2 is blank.
EXPECTED RESULTS: Page 2 should have the text "Page 2".
This happens because we attempt to reflow cb at the bottom of page 1, and it decides that its first child (a zero-height placeholder, and the corresponding abspos frame) fits there, when really we'd like that placeholder and abspos frame (and cb as a whole) to get pushed to page 2.
(In some cases we do end up painting the abspos on page 2 just fine, particularly if you remove overflow:hidden from cb to avoid clipping at the bottom of its continuation on page 1; that lets the "fragmentation fallback" codepath kick in and get painted-content-oveflowing-from-page-1-to-paint-on-page-2. But really we don't want the "Page 2" text to get a frame on page 1 at all.)
|
|
Reporter | |
Comment 3•2 years ago
|
||
This is a testcase with a bunch of empty zero-height divs, some of which have break-after:page. Chrome and Firefox both render this as 1 page right now.
When fixing this bug, we might want to be careful to preserve our behavior on this testcase. (Though maybe it's more-correct to add pagebreaks here?)
|
|
Reporter | |
Comment 4•2 years ago
•
|
||
Here's a variant of the breaktest, to demonstrate that Firefox and Chrome still nerf break-after:page on empty divs at the top of the page, even if those empty divs have nonzero CSS margin.
In print-preview, this produces two pages in Firefox and Chrome right now. If I add any content (or any padding or border or height) to one of the divs near the start, then suddenly it renders many more pages.
Anyway, as with testcase 3: when fixing this bug, we might want to try to avoid changing behavior on this testcase, to avoid unnecessarily taking a step away from interop, unless it's an unavoidable & clearly-justifiable outcome.
|
Assignee | |
Comment 5•2 years ago
|
||
|
||
Comment 8•2 years ago
|
||
| bugherder | ||
|
|
Assignee | |
Comment 10•2 years ago
•
|
||
After some investigation, it seems that this bug has been present in Firefox for a very long time.
I spent some more time checking that the effects of this patch are correct, and it seems like they are even where might not be obvious. I had considered that it might be necessary to add a check that we don't do this pushing when the previous frame was an nsPageBreakFrame, but it seems like that is unnecessary.
This bug could occur in any situation resembling:
<div style="break-after: page">Page 1</div>
<div style="height: 0">Zero-height content</div>
<div>Page 2</div>
Where before, the zero-height div wasn't automatically pushed to the next page. This sometimes worked properly, (as in that specific example). But if, for instance, several zero-height divs were present that also had break-after: page set, all of those would appear on the page 2:
<div style="break-after: page">Page 1</div>
<div style="break-after: page; height: 0">Zero-height content 1</div>
<div style="break-after: page; height: 0">Zero-height content 2</div>
<div>Page 4</div>
The correct behavior is to put the zero-height divs each on their own pages, and the Page 4 div on the fourth page. However, before this patch Firefox would put all the zero-height content on the second page, along with Page 4 (actually making it page 2).
This is all caused by this behavior: https://searchfox.org/mozilla-central/rev/2f5b657343ce18e4b8a56417f707022550f4b742/layout/generic/nsPageFrame.cpp#1000-1002 This patch in part undoes the effect of rounding the height down. Before, the zero-height divs would have fit into the space after the page-break-frame, and only on further reflow would those frames have generated new continuations. I spent some time trying to understand why this works this way in the first place, since this change seems to be more interoperable and more correct for the spec, and didn't break any tests. I think that this bug is either an unintentional, or if intentional is now a totally obsolete side effect of the choice to round down the height. Rounding down the height has at least one important effect: it allows coalescing page-breaks, for instance between break-after: page and break-before: page, or in a case such as:
<div style="break-after: page">
<div style="break-after: page">
</div>
</div>
Where otherwise, both the inner and outer div would cause separate page-breaks.
Description
•