Fix doc.list() throwing on a numbered or lettered list with align center or right

[MaxFreedomPollard]

What kind of change does this PR introduce?

Bug fix. doc.list(['alpha', 'beta'], { listType: 'numbered', align: 'center' }) throws Error: unsupported number: NaN instead of producing a PDF. The same happens for align: 'right' and for listType: 'lettered'. align: 'justify' on its own does not throw, but with underline or strike it throws on the first label, and with link or goTo it gives that label a zero width rectangle, /Rect [72 708.9 72 720]. A justified list also writes a bogus word spacing into every label after the first, taken from the previous line: a one word label has nothing to stretch, so the printed number does not move, but the bytes change and that spacing feeds the width that underline, strike, link and goTo use. Bullet lists and plain doc.text() with the same alignment are fine. Separately, and with no alignment option involved at all, the underline, strike, link and goTo of every label after the first are sized from the previous item's text: doc.list(['alpha', 'beta'], { listType: 'numbered', link: 'http://example.com' }) gives the label 2. a rectangle of /Rect [72 695.028 101.351999 706.128] when that label is 10.008 points wide. All of it comes from the same line. No open issue covers any of it.

The bug

lib/mixins/text.js, in the firstLine handler that list() installs on its LineWrapper:

case 'numbered':
case 'lettered':
  var text = formatListLabel(numbers[i - 1], listType);
  this._fragment(text, this.x - indent, this.y, options);  // the wrapper's own options
  break;

options here is the object the LineWrapper was constructed with and writes into. lib/line_wrapper.js sets options.lineWidth, options.textWidth and options.wordCount inside emitLine(), and firstLine is emitted before the first emitLine(), so on the first item all three are still undefined when the label is drawn.

_fragment then applies the paragraph alignment to that label:

if (options.width) {
  switch (align) {
    case 'right':
      textWidth = this.widthOfString(text.replace(/\s+$/, ''), options);
      x += options.lineWidth - textWidth;   // options.lineWidth is undefined
      break;
    case 'center':
      x += options.lineWidth / 2 - options.textWidth / 2;   // both undefined
      break;

x becomes NaN and PDFObject.number throws. In normal use options.width is truthy here without the caller setting anything: _initOptions defaults it to this.page.width - this.x - this.page.margins.right whenever lineBreak !== false. It is falsy only if the caller passes lineBreak: false, or the list starts at the right margin, where the same function clamps that default to 0.

justify reaches the second label only by accident. It derives a word spacing from options.lineWidth, so Math.max(0, NaN) leaves wordSpacing as NaN, and NaN is falsy at the point where the spacing would be encoded, so the glyphs are still written. renderedWidth is not so lucky: it adds wordSpacing * (measuredWordCount - 1) to the measured width, and NaN * 0 is NaN. That NaN reaches lineTo for underline and strike and throws on the very first label. For link and goTo it goes through _convertRect, where NaN < 0 and NaN > 0 are both false, so the annotation collapses to a zero width rectangle with no error at all.

From the second label on, lineWidth holds the width of the previous line and the spacing is a real number. Master writes the second label of a justified numbered list as [<322e> -36666] TJ where the fix writes [<322e> 0] TJ. The -36666 is a trailing adjustment after the only glyph run, so the number still prints at x=72 either way; what is wrong is the spacing in the stream, and once again the renderedWidth it feeds.

The last part of the defect needs no alignment. Once the first item has been wrapped, the shared object holds that item's textWidth, and it is still there when the next item's label is drawn. _fragment computes renderedWidth from options.textWidth ?? this.widthOfString(...), so the stale value wins over measuring the label. That width is what drives the link and goTo rectangles and the underline and strike lines.

Reproduction of the throw:

const doc = new PDFDocument({ compress: false });
doc.list(['alpha', 'beta'], { listType: 'numbered', align: 'center' });
doc.end();

On master: Error: unsupported number: NaN, thrown at lib/object.js:170, from _fragment at lib/mixins/text.js:627, from the firstLine handler at lib/mixins/text.js:355.

Reproduction of the width bug, on the default path:

doc.list(['alpha', 'beta'], { listType: 'numbered', underline: true });

Master underlines the label 2. from x=72 to x=101.351999, which is 72 plus the width of alpha, instead of to x=82.008. Swap underline for link and the same list writes /Rect [72 695.028 101.351999 706.128] for that label, where the correct rectangle is /Rect [72 695.028 82.008 706.128]. The first label is right in both cases, because there is no earlier item to inherit from.

The fix

list() builds one options object for the label, once, and passes that to the label fragment:

const labelOptions = { ...options, align: 'left' };
delete labelOptions.lineWidth;
delete labelOptions.textWidth;
delete labelOptions.wordCount;

Forcing align: 'left' stops the label taking the paragraph alignment. Dropping the three line measurements keeps any wrapper's writes out of the label options, so the existing ?? fallback in _fragment measures the label itself. Both parts are needed: the alignment alone still leaves a stale width behind, and the measurements alone still centre the number on top of the body text.

The three deletes also close the one case a plain copy does not reach. A list placed directly after a continued: true text run inherits that run's textWidth through _initOptions, which copies every key of this._textOptions that the new options leave undefined, and the earlier wrapper had already written its measurements into that object. With align: 'left' and nothing else, doc.text(long, { continued: true }) followed by an underlined numbered list underlines both labels to the width of the continued run, and master underlines the first to that width and the second to the width of item one. With the deletes both labels underline from 72 to 82.008, which is the label.

What does not change: bullet lists, and numbered and lettered lists that use the default alignment and none of underline, strike, link or goTo. I compared the full output of a nested numbered list, a lettered list, a bullet list and a bullet list with align: 'center' before and after, and it is byte identical. Labels of numbered and lettered lists that do use one of those four options, or align: 'justify', do change from the second label onwards, and they change because master was wrong. No visual test passes align to list(), so the tests/visual snapshots are unaffected.

One alternative worth ruling out, since it is the shape of the earlier fix in #1784: this could be patched inside _fragment instead, by falling back to a measured lineWidth the way that PR did for textWidth. That would leave the alignment applied to the label, which centers the number inside the item's line width and so prints it on top of the centered body text, and pushes it to the right margin for align: 'right'. The label is not one of the wrapped lines, so it should not take the paragraph alignment at all.

Testing

Seven cases added to tests/unit/list.spec.js.

A test.each over numbered and lettered crossed with center and right asserts the label stays at the item indent (x=72) while the body is aligned (x=300.324 centered, x=510.648 right). All four fail on master with unsupported number: NaN.

One test asserts that the default alignment still puts the label at 72 and the body at 90 for both items. It passes before and after, and exists to guard the path that must not move.

One test covers the justify path, align: 'justify' with underline: true. On master it throws unsupported number: NaN out of lineTo on the first label. With the fix both labels are underlined from 72 to 82.008, so the second label's width is pinned too.

One test asserts the link annotation of the second label. On master the diff is:

- /Rect [72 695.028 82.008 706.128]
+ /Rect [72 695.028 101.351999 706.128]

npx vitest run tests/unit/list.spec.js gives 6 failed and 3 passed with lib/mixins/text.js from master, and 9 passed on the branch. npx vitest run tests/unit gives 42 files and 461 tests passed. npm run prettier reports that all matched files use Prettier code style and npm run lint exits clean. I could not run tests/visual locally, so CI will be the first run of that suite.

Checklist:

• Unit Tests
• Documentation N/A
• Update CHANGELOG.md
• Ready to be merged

[blikblum]

Suggested change

	const labelOptions = { ...options, align: 'left' };
	delete labelOptions.lineWidth;
	delete labelOptions.textWidth;
	delete labelOptions.wordCount;
	const {
	lineWidth,
	textWidth,
	wordCount,
	...labelOptions
	} = options;
	labelOptions.align = 'left';

if lint complains suppress the warning in the line