Put the page back after measuring a page-break-inside:avoid block (mpdf/mpdf#533 #570 #785 #1131 #1666 #1805 #2075 #322 #1801) - #38
Draft
jakejackson1 wants to merge 12 commits into
Conversation
…(mirrors mpdf#604) A block with page-break-inside:avoid is laid out twice: once to find out how tall it is, and then again for real once BlockTag::close() has decided where it goes. Nothing paints on the measuring pass - BaseWriter::write() throws the output away and PaintDivBB() returns straight back out - because the unwind afterwards only throws away the pages after the one the block started on. Whatever reached that starting page stays there. _tableWrite() splices the table backgrounds straight into $this->pages[$this->page] without asking, so a table inside such a block had its background painted on the starting page during the measuring pass, and then again in its real place after the block had moved to the next page. The reporter saw a coloured band left behind at the bottom of the previous page. It takes a table before the block to show up, because that is what leaves a placeholder on the starting page for the backgrounds to be spliced into. Skip the print while keep_block_together is set, the way everything else that paints already does. The collected backgrounds are still cleared, so the real pass starts from nothing and draws them where the table ended up. All twenty snapshots are unchanged. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Every table writes a ___TABLE___BACKGROUNDS placeholder into the page and, when it ends, splices its backgrounds in behind it. The splice only ran when the table actually had backgrounds, so a table with none left its placeholder sitting in the page - and because the splice is a plain preg_replace over the whole page, the next table's backgrounds went behind that stale placeholder as well as behind its own. Five plain tables in front of a coloured one had its background painted six times. That is invisible while the fill is opaque, but a translucent background comes out darker than it was asked to be, a background image is drawn and drawn again, and the page content stream carries a copy of the whole lot each time. Take the placeholder out at the end of every level 1 table, whether or not anything was put behind it. The two preg_replace calls per buffer collapse into one str_replace, which is also no longer at the mercy of a $ or a backslash turning up in the background operators. tests/data/snapshots/table.pdf is regenerated: its fixture has a plain table in front of a coloured one, so three pixels of antialiasing along the table's right edge shift now that the background underneath is painted once. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
jakejackson1
marked this pull request as draft
September 7, 2026 10:39
Member
Author
|
Handle all page-break-inside bug fixes together |
Stop a table with no background leaving the next one painted twice
… block A block with page-break-inside:avoid is laid out once to see where it ends and, if that ran onto another page, the pages it made are thrown away and it is laid out again from where it started. The block itself paints nothing on the measuring pass, but AddPage() switches that discard off while it runs, so on the page the block started from - the one page the unwind keeps - it wrote the footer, the watermark and the page and body backgrounds, and _beginpage() then selected the next page's @page rules, margins, header and footer. The unwind put back y and nothing else. When the block was laid out for real the same page was finished again, under the wrong rules: a watermark drawn twice (mpdf#533), page 2's footer and background on page 1 (mpdf#1131), the @page background painted over the block backgrounds collected before it (mpdf#1805), and the table background of mpdf#570 that the previous commit dealt with on its own. Record the page-level state when the block opens and put it back when the block unwinds: the @page selection and everything it sets, the header and footer slots, the arrays keyed by page (which also drops the pages the measuring pass made), the backgrounds collected for the page, the patterns and page-number substitutions added since, the float bookkeeping, and the enclosing blocks, which had painted their borders down to the foot of the page and remembered doing so. Two more things the measuring pass got wrong. A float inside the block cancelled keep-together but left the block flagged, so the float was written by both passes, at a y the unwind had zeroed (mpdf#785); a floated block is now never flagged in the first place, and a float inside a kept block is measured with the rest of it. And a block inside a header, footer or fixed-position block has no page to break, and its output is buffered rather than discarded, so it was written twice (the cases in mpdf#1131's thread, and the likely shape of mpdf#322); the measuring pass is not started while output is buffered. The tests read the page content streams; the helpers for that are now shared with TableBackgroundTest. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…pdf#2075) When useSubstitutions finds a character the current font lacks, it splices a span in the backup font into the token array after the current token and trims the working string down to the text before it. The token itself kept the whole text. That is invisible on a single parse, but a page-break-inside:avoid block is parsed twice, and the second parse read the untrimmed token, printed all of it, and then the span and the tail again: "HÄẞLICH" came out as "HÄ[]LICHẞLICH". SubstituteCharsSIP() already wrote the trim back into the token. The seven other splice sites now do the same. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…pdf#1801) page-break-after:avoid is approximated by demanding room for one more line as tall as the block after it, once per line in Cell() and again when the block closes. A block taller than half a page can never have that on any page, so each check pushed it on to yet another one, and a wide image arrived after two blank pages. Skip the look-ahead when even a fresh page could not satisfy it. A heading with only its own height left on the page still moves. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A caption is written as a block above its table, so when a table with page-break-inside:avoid moved to the next page the caption stayed where it was. That is keep-with-next, which mPDF only has for headings and only under use_kwt. With it on, a caption above its table is now marked keep-with-table the same way a heading is, and moves with it. Without use_kwt nothing changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
One four-page document for the page-break-inside:avoid fixes: kept-together cards over a page break under a first-page background, footer and watermark; a framed section whose signature block, with floats, moves whole; and a captioned table kept with its caption under use_kwt. Rendered before the fixes, the signature line is printed twice on page 2. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ot for the avoid block A review pass over the whole PR. _tableWrite() spliced table backgrounds behind their placeholder at two sites: at the end of a level 1 table, in the new one-shot form, and when a table breaks across pages, still in the old two-preg_replace form that only ran when there were backgrounds and so left the placeholder in place when there were none. Both now go through spendTableBackgrounds(), which takes the buffer the placeholder was written to. The guard for a block that is only being measured narrows to the one thing that has a side effect, PrintTableBackgrounds() registering patterns and gradients; the placeholder is spent either way, since BlockTag puts the page back afterwards. BlockTag reuses the float it already worked out for the page-break-inside gate in the two float branches, and restores the saved properties through the existing restoreStateSnapshot(). The state list explains its three groups inline rather than in a docblock that had to be read against blank lines. The two snapshot documents for a moved avoid block become one: the plain table and the coloured panels from page-break-avoid.pdf now sit in front of and inside the cards of page-break-avoid-state.pdf, which is regenerated. Two pairs of TableBackgroundTest cases that rendered the same document are merged, and the test trait cleans up after every document it outputs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The float cases the earlier tests left out: a float taller than what is left of the page, so the block moves for the float's sake; a float taller than a page, so the block cannot be kept together and breaks like any other; floated images, which go through their own buffer; text keeping its indent beside a float after the move; a float still open when the block starts, which used to be lost outright; and a float inside a block inside columns, where keep-together is off. The three float tests already in PageBreakInsideAvoidTest move into the new class with them. tests/data/snapshots/page-break-avoid-floats.pdf puts the four shapes on one document. Rendered before the fix, every float inside a moved block is written twice and the column floated before a block is missing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A page takes about twenty-nine of the test filler paragraphs, so the thirty used to leave "room for a few more" spilled onto page 2 by themselves and the block that was meant to move simply started there. Twenty-two leave a few lines free and not enough for the block. Run against gravitypdf, eight of the eleven cases in PageBreakInsideAvoidTest and six of the ten float cases now fail; the rest are controls, or guard the rollback against dropping what it restores. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
page-break-inside:avoid on a floated block was ignored: the gate excluded floats, which only preserved what mPDF always did, and lifting the exclusion changed nothing, because a float's close puts the position back to where the float started before the keep-together check looks at it. The block was never seen to have left its page, so it split at the foot of it like any float. Record where the float ended before its close resets the position, and decide the move from that. A floated block that does not fit now moves whole to the next page when it would fit there, with the text that wraps beside it; one taller than a page still breaks, as any block does. Plain floats are unchanged. Nine more float cases cover a right float, left and right floats moving together, one that fits staying while one that does not moves, a border and background painted only where the block ends up, a float inside the block, an image and a table inside it, and a block inside a bordered container that continues across the page. The float snapshot gains a kept floated block and a pair of them with a border, a background, a picture and a float of their own. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A
page-break-inside: avoidblock is laid out twice: once to measure it, and, if that ran onto another page, again from where it started after the pages it made are thrown away. The measuring pass never wrote the block itself, but everything mPDF does when a page ends still ran, and the page it started on was the one page the unwind kept. This PR puts that page back as it was, and clears up the issues that had gathered around the symptom.Closes mpdf#533 (watermark drawn twice), mpdf#570 (table background left behind), mpdf#1131 (page 2's footer and background on page 1), mpdf#1805 (
@pagebackground painted over block backgrounds), mpdf#322 (content written twice or not at all), mpdf#785 (a float inside the block laid out twice), mpdf#2075 (useSubstitutionsduplicating text), mpdf#1801 (blank pages before a tall block withpage-break-after: avoid) and, withuse_kwton, mpdf#1666 (a caption left behind by its table).The first commit mirrors mpdf#604 and is what this PR opened with; the five that follow are new, one per cause, and a last one applies a review pass over the whole PR (the two table-background placeholder splices in
_tableWrite()become onespendTableBackgrounds()helper, which skips only the pattern registration while a block is measured; the two snapshot documents for the avoid block become one).What the measuring pass left behind.
AddPage()turns the discard off while it runs, so on the page the block started from it wrote the footer, the watermark and the page and body backgrounds, and_beginpage()then selected the next page's@pagerules, margins, header and footer. The unwind deleted the later pages and resety, and nothing else. When the block was laid out for real the same page was finished a second time, with the wrong rules.BlockTagnow records the page-level state when the block opens and restores it when the block unwinds: the page's content and the backgrounds collected for it, the@pageselection, geometry and margins, the header and footer choice, float state, the patterns and page-number substitutions added since, and the enclosing blocks (which had already painted their borders down to the foot of the page and remembered doing so).Three of the nine had causes of their own. A float inside the block cancelled keep-together but left the block flagged, so the float was written by both passes at a
ythe unwind had zeroed (mpdf#785); a floated block is now never flagged in the first place, and a float inside a kept block is measured with the rest of it. The character substitution trimmed its working string but not the token it came from, so the second parse printed the whole text and then the spliced span and the tail again (mpdf#2075). And mpdf#1801 is notpage-break-insideat all:page-break-after: avoidasks for room for one more line as tall as the block, which a tall image can never have on any page, so each check pushed it onto another one.A caption is a different problem again (mpdf#1666). It is a block written before its table, so when a table with
page-break-inside: avoidmoves, the caption needs keep-with-next, not replay state. Underuse_kwta top caption is now carried with its table the way a heading is. Withoutuse_kwtit stays behind, as before.Floated blocks now honour it too.
page-break-inside: avoidon a floated block was ignored: a float's close puts the position back to where the float started before the keep-together check looks at it, so the block was never seen to have left its page and split at the foot of it like any float. The float's end is now recorded before that reset and the move decided from it; a floated block that does not fit moves whole, with the text that wraps beside it, and one taller than a page still breaks. Plain floats are unchanged.The measuring pass is also not started while output is buffered, which is the case inside headers, footers and fixed-position blocks: there is no page to break there, and buffered output is not discarded, so the block was written twice (the cases reported in mpdf#1131's thread).
Try it
Before: page 1 is green with "Footer 2" and a darker watermark than page 2. After: page 1 is red with "Footer 1", and the watermark is the same weight on both pages.
Test plan
tests/Mpdf/PageBreakInsideAvoidTest.php, eleven cases, eight of which fail ongravitypdf; the other three are a control for thepage-break-afterchange and two guards that the rollback keeps what it must (the page background collected before the block, the enclosing border). The page-stream helpers move into aPageStreamstrait shared withTableBackgroundTest. Cases: one watermark per page; the first page keeps its own@pagerules and footer; the@pagebackground is painted once and under the block backgrounds; a background collected before the block is still painted; an enclosing border is painted on the first page after the block moves; a kept-together paragraph inside a fixed-position block or a footer is written once; a substituted character is not duplicated; a caption moves with its table underuse_kwtand stays behind without; a tall block withpage-break-after: avoidno longer pushes blank pages, and a heading with only its own height left still moves.tests/Mpdf/PageBreakInsideAvoidFloatTest.php, nineteen cases for floats (the first ten: six fail ongravitypdf, floated images were never duplicated, and two are controls; the nine floated-block cases all fail there): a float inside the block is laid out once and moves with it; text keeps its indent beside the float after the move; a float taller than the space left moves the block; a float taller than a page makes the block break, once; floated images move with the block and are drawn once, moving or not; a float still open when the block starts is not lost (it used to be); a float inside a block inside columns, where keep-together is off; and for floated blocks themselves: one that does not fit moves whole and one that fits stays, one taller than a page still breaks, a right float, left and right moving together, one fitting while the other moves, a border and background painted only where the block lands, a float inside it, an image and a table inside it, and one inside a bordered container that continues across the page.tests/Snapshots/PageBreakAvoidStateSnapshotTest.phpputs all of it on one four-page document (tests/data/snapshots/page-break-avoid-state.pdf): a plain table, then kept-together cards over a page break under a first-page background, footer and watermark, the last card carrying coloured table panels; a framed section whose signature block, with floats, moves whole; and a captioned table kept with its caption. Rendered ongravitypdfthe red panel is also painted at the foot of page 1 and the signature line is printed twice on page 2.tests/Snapshots/PageBreakAvoidFloatsSnapshotTest.php(tests/data/snapshots/page-break-avoid-floats.pdf, eight pages): a floated text column beside wrapped text in a block that moves whole, floated pictures in a block that moves, a column floated before a block opens, a block a floated column makes taller than a page, a floated block that is itself kept together, and a pair of kept floated blocks with a border, a background, a picture and a float of their own. Each moved block leaves free space at the foot of the page it left. Ongravitypdfevery float inside a moved block is drawn twice and the column floated before a block is missing.tests/Mpdf/TableBackgroundTest.php(six cases) andtests/Snapshots/TableBackgroundSnapshotTest.phpfrom the first commits.gravitypdfare byte-identical once the creation date, the/ID, the link annotations'/Mtimestamps and the git hash in/Producerare normalised.composer test— 1158 tests, 2821 assertions, up from 1130 / 2653.composer csclean.More info — what is and is not in the snapshot
The state list in
BlockTag::$pageStateis everythingAddPage(),_beginpage()andFooter()assign that outlives the page, in three groups: what_beginpage()selects for the next page (page_box, body background, crop marks, orientation, page size, margins and the derivedpgwidth,PageBreakTrigger,MarginCorrection,y0, the four header and footer slots and the deferred first-page ones); whatAddPage()finishes the current page with, which it only writes because it switches the discard off while it runs (pageBackgrounds,patterns,PageNumSubstitutions, and the arrays keyed by page:pages,pageoutput,saveHTMLHeader,saveHTMLFooter,PageLinks,PageAnnots,pageDim,OrientationChanges, put back whole, which also discards the pages the measuring pass made); and the float bookkeeping (blockContext,floatmargins,floatbuffer,floatDivs). The enclosingblklevels are copied one by one:open()holds the level below by reference while the snapshot is taken, and a copy of the whole array would keep sharing it.All of it is copy-on-write until the measuring pass actually breaks a page, and measured against
gravitypdfon 3,000 kept paragraphs it costs about 14 µs per block and no peak memory.Not snapshotted, deliberately:
listcounter(the block's own<li>is not re-counted on the second parse, so restoring the counter would put it one behind),subPos(the substitution splices are in place and the write-back makes them idempotent), fonts and colours (Reset()and thepageoutputreset already re-emit them), and document-level caches such asimagesand FPDI's imported pages, where a duplicate is harmless.The
bb_paintedandmarginCorrectedunsets the old loop did on the block being closed are gone: that block is unset outright a few lines later, and the enclosing blocks come back from the snapshot. So are theinitial_y0/initial_x0/initial_startpagefields: their only reader was the loop the snapshot replaces.