Skip to content

Fix indentAllLines applying the indent more than once - #1767

Merged
blikblum merged 1 commit into
foliojs:masterfrom
MahathirMohammadShuvo:fix-indent-all-lines-compounding
Aug 18, 2026
Merged

Fix indentAllLines applying the indent more than once#1767
blikblum merged 1 commit into
foliojs:masterfrom
MahathirMohammadShuvo:fix-indent-all-lines-compounding

Conversation

@MahathirMohammadShuvo

@MahathirMohammadShuvo MahathirMohammadShuvo commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix. Fixes #1606.

indentAllLines keeps the indent in effect after the first line, but nothing tracked whether it had already been applied. nextSection() re-applied it on every page break, and the firstLine handler re-applied it on every paragraph, so the indent accumulated instead of staying put.

With width: 300, indent: 15, indentAllLines: true:

before after
x on each paragraph of a multi-paragraph wrap 15, 30, 45 … 15 on every paragraph
lineWidth after successive page breaks 285 → 270 → 255 … 285 on every page
page break before the first line (orphan check) indent applied twice applied once

lineWidth carries over between pages while addPage() resets document.x, so subtracting the indent again on every page break narrowed the text column further and further. With enough pages it reaches zero, no word fits on a line, and pagination runs away — that reproduces as an out-of-memory crash.

This is a regression from the fix for #1686, which added the indent restore in nextSection() without accounting for the firstLine handler having already applied it.

Continued text (#1606)

Continued segments inherit the previous call's options, so a follow-on text() is also indentAllLines and took the same early return — which meant the continuation offset was never taken back off and every line of the continued segment sat at the offset where the previous segment happened to end:

doc.text(long, { width: 300, indent: 15, indentAllLines: true, continued: true });
doc.text(long, { width: 300 });
line before after
first line of the continued segment x = 195.1 x = 195.1 (carries on from the previous segment)
every line after it x = 195.1, lineWidth 104.9 x = 15, lineWidth 285

The continuation offset belongs to the first line of a segment only, so it is now taken back off after that line while the paragraph indent stays on.

Checklist:

  • Unit Tests
  • Documentation — N/A, no API or option change
  • Update CHANGELOG.md
  • Ready to be merged

Four regression tests added to tests/unit/line_wrapper.spec.js, covering paragraphs, page breaks, the orphan check path where the page break runs before firstLine, and continued text. All four fail against the current lib/ and pass with the fix. Full unit suite passes, 384/384.

@MahathirMohammadShuvo
MahathirMohammadShuvo force-pushed the fix-indent-all-lines-compounding branch from e088074 to ee95926 Compare August 18, 2026 08:28
@MahathirMohammadShuvo

Copy link
Copy Markdown
Contributor Author

Updated the branch. While checking this against continued text I found the same root cause behind #1606, so the scope grew slightly: continued segments inherit the previous call's options, so the follow-on text() is also indentAllLines and took the same early return, leaving the continuation offset applied to every line of the segment instead of just the first.

The description above has the before/after numbers. Fourth regression test added for it; the suite is green at 384/384.

@blikblum

Copy link
Copy Markdown
Member

Please fix the conflict

indentAllLines keeps the indent in effect after the first line, but nothing
tracked that it had already been applied. nextSection() re-applied it on every
page break, and the firstLine handler re-applied it on every paragraph, so the
indent accumulated.

Paragraphs stepped progressively further right, and lineWidth - which carries
over between pages while addPage() resets document.x - shrank by the indent on
every page break, until eventually no word could fit on a line and pagination
ran away.

Track whether the indent is currently applied and apply it only once. On a page
break restore document.x alone, since lineWidth already accounts for the indent.

Adds regression tests covering paragraphs, page breaks, and the orphan check
path where the page break runs before firstLine.
@MahathirMohammadShuvo
MahathirMohammadShuvo force-pushed the fix-indent-all-lines-compounding branch from ee95926 to 5174d37 Compare August 18, 2026 11:55
@blikblum
blikblum merged commit a7a5363 into foliojs:master Aug 18, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

indentAllLines: true does not work when continued: true is used in a paragraph that includes rich text formatting in pdfkit

2 participants