From 5174d37b34ce251b06deb92e03337d3e68b5244b Mon Sep 17 00:00:00 2001 From: Mahathir Mohammad Shuvo Date: Tue, 18 Aug 2026 14:04:39 +0600 Subject: [PATCH] Fix indentAllLines applying the indent more than once 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. --- CHANGELOG.md | 1 + lib/line_wrapper.js | 38 +++++++++++-- tests/unit/line_wrapper.spec.js | 98 +++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e441f5ec..eded8f7ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Persist font options when adding a new page. Fixes #1739 - Use `Uint8Array` instead of Node's `Buffer` internally - Fix `date` text field formatting emitting invalid JavaScript, so the format was never applied. Fixes #1546 +- Fix `indentAllLines` applying the indent again on every paragraph and every page break, and keep it applied across continued text. Fixes #1606 ### [v0.19.1] - 2026-06-10 diff --git a/lib/line_wrapper.js b/lib/line_wrapper.js index 38be280f1..ee2f26bf6 100644 --- a/lib/line_wrapper.js +++ b/lib/line_wrapper.js @@ -31,6 +31,7 @@ class LineWrapper extends EventEmitter { this.column = 1; this.ellipsis = options.ellipsis; this.continuedX = 0; + this.indentApplied = false; this.features = options.features; // calculate the maximum Y position the text can appear at @@ -46,13 +47,33 @@ class LineWrapper extends EventEmitter { // if this is the first line of the text segment, and // we're continuing where we left off, indent that much // otherwise use the user specified indent option - const indent = this.continuedX || this.indent; - this.document.x += indent; - this.lineWidth -= indent; + // a continued segment carries on from where the previous one ended, and + // that offset belongs to its first line only + const continuation = this.continuedX; + const indent = continuation || this.indent; + + // With indentAllLines the indent is never taken back off after the first + // line, so it may already be in effect: applied by an earlier paragraph in + // this same wrap, or restored by nextSection() after a page break. + // Applying it a second time would push x further right and shrink + // lineWidth again on every paragraph and every page. + if (continuation || !this.indentApplied) { + this.document.x += indent; + this.lineWidth -= indent; + this.indentApplied = true; + } // if indentAllLines is set to true // we're not resetting the indentation for this paragraph after the first line if (options.indentAllLines) { + // the paragraph indent stays on, but the continuation offset is only + // for the first line of this segment + if (continuation) { + this.once('line', () => { + this.document.x -= continuation; + this.lineWidth += continuation; + }); + } return; } @@ -60,6 +81,7 @@ class LineWrapper extends EventEmitter { this.once('line', () => { this.document.x -= indent; this.lineWidth += indent; + this.indentApplied = false; if (options.continued && !this.continuedX) { this.continuedX = this.indent; } @@ -351,9 +373,13 @@ class LineWrapper extends EventEmitter { this.startY = this.document.page.margins.top; this.maxY = this.document.page.maxY(); if (this.indentAllLines) { - const indent = this.continuedX || this.indent; - this.document.x += indent; - this.lineWidth -= indent; + // addPage() has reset document.x to the page margin, but lineWidth is + // carried over from the previous page and already accounts for the + // indent, so only x needs restoring here. If the indent is not in + // effect yet the firstLine handler will apply both. + if (this.indentApplied) { + this.document.x += this.continuedX || this.indent; + } } else { this.document.x = this.startX; } diff --git a/tests/unit/line_wrapper.spec.js b/tests/unit/line_wrapper.spec.js index 7c44cfa3f..11c83dbaf 100644 --- a/tests/unit/line_wrapper.spec.js +++ b/tests/unit/line_wrapper.spec.js @@ -177,4 +177,102 @@ describe('LineWrapper', () => { wrapper.wrap('B', {}); expect(pageBreaks).toBeGreaterThanOrEqual(1); }); + + test('indentAllLines indents every paragraph by the same amount', () => { + const options = { width: 300, indent: 50, indentAllLines: true }; + const wrapper = new LineWrapper(document, options); + + const geometry = []; + wrapper.on('line', () => { + geometry.push({ x: document.x, lineWidth: wrapper.lineWidth }); + document.y += document.currentLineHeight(true); + }); + + wrapper.wrap('alpha\nbravo\ncharlie', options); + + // the indent is only taken back off after the first line when + // indentAllLines is false, so each paragraph used to add another indent + // on top of the previous one + expect(geometry).toEqual([ + { x: 50, lineWidth: 250 }, + { x: 50, lineWidth: 250 }, + { x: 50, lineWidth: 250 }, + ]); + }); + + test('indentAllLines keeps the same line width across page breaks', () => { + const options = { width: 300, indent: 50, indentAllLines: true }; + const wrapper = new LineWrapper(document, options); + + const breaks = []; + wrapper.on('pageBreak', () => { + breaks.push({ x: document.x, lineWidth: wrapper.lineWidth }); + }); + wrapper.on('line', () => { + document.y += document.currentLineHeight(true); + }); + + wrapper.wrap('word '.repeat(1200), options); + + // lineWidth carries over between pages, so subtracting the indent again + // on every page break narrowed the text column further and further + expect(breaks.length).toBeGreaterThan(1); + breaks.forEach((geometry) => { + expect(geometry).toEqual({ x: 50, lineWidth: 250 }); + }); + }); + + test('indentAllLines does not double the indent when a page break comes first', () => { + const options = { width: 300, indent: 50, indentAllLines: true }; + // start near the bottom so the orphan check starts a new page before the + // first line is emitted, running the page break before firstLine + document.y = document.page.maxY() - 2; + const wrapper = new LineWrapper(document, options); + + let firstLineGeometry = null; + wrapper.on('line', () => { + if (firstLineGeometry === null) { + firstLineGeometry = { x: document.x, lineWidth: wrapper.lineWidth }; + } + document.y += document.currentLineHeight(true); + }); + + wrapper.wrap('word '.repeat(20), options); + + expect(firstLineGeometry).toEqual({ x: 50, lineWidth: 250 }); + }); + + test('indentAllLines keeps the indent when the text is continued', () => { + const geometry = []; + const line = document._line; + document._line = function (text, options, wrapper) { + geometry.push({ x: document.x, lineWidth: wrapper && wrapper.lineWidth }); + return line.call(this, text, options, wrapper); + }; + + const sentence = 'lorem ipsum dolor sit amet consectetur adipiscing '; + const paragraph = sentence.repeat(4); + document.text(paragraph, { + width: 300, + indent: 15, + indentAllLines: true, + continued: true, + }); + const continuedFrom = geometry.length; + document.text(paragraph, { width: 300 }); + + // continued options are inherited, so the second call is also + // indentAllLines: its first line carries on from where the previous + // segment ended, and every line after that keeps the paragraph indent + geometry.slice(0, continuedFrom).forEach((line) => { + expect(line).toEqual({ x: 15, lineWidth: 285 }); + }); + + const continued = geometry.slice(continuedFrom); + expect(continued.length).toBeGreaterThan(1); + expect(continued[0].x).toBeGreaterThan(15); + continued.slice(1).forEach((line) => { + expect(line).toEqual({ x: 15, lineWidth: 285 }); + }); + }); });