Add widget page support for placeholder-plain

[sestr]

The widget for a signature could only be placed on the first page of a PDF document with the placeholder-plain package. However, it should also be possible to insert the widget on any other page. I made this possible by adding the widgetPage parameter to the configuration object of the plainAddPlaceholder method.

[coveralls]

coverage: 99.319% (-0.3%) from 99.636%
when pulling 0f20522 on sestr:feature/widget-page
into 4a6a2a4 on vbuch:develop.

[vbuch]

This PR seems like something completely valid. I don't know why it was never reviewed. @sestr any interest in rebasing so we can get this merged?

The only possibility I think would be even better is to support the signature being shown on every page. Not part of this PR in any way but an interesting feature to have. It should be doable as long as there are /Widgets on every page all linking to the same /Sig.

[sestr]

No problem @vbuch, I just did a rebase.

Adding a signature to every page is indeed an interesting idea for an additional feature. This could be used, for example, to initial each page of a contract.

[coveralls]

coverage: 99.47% (-0.5%) from 100.0%
when pulling 987cc53 on sestr:feature/widget-page
into f54ca67 on vbuch:develop.

[Copilot]

Pull Request Overview

This PR adds support for specifying a target page when inserting a signature widget via plainAddPlaceholder.

• Introduces a new widgetPage parameter in the configuration and JSDoc for plainAddPlaceholder
• Updates plainAddPlaceholder to pass widgetPage through to getPageRef
• Enhances getPageRef to accept an optional pageNumber, split out page references, and validate bounds

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

File	Description
packages/placeholder-plain/src/plainAddPlaceholder.js	Added widgetPage to JSDoc, function signature defaults, and parameter propagation
packages/placeholder-plain/src/getPageRef.js	Extended getPageRef signature, reworked page reference parsing, and added bounds checks

Comments suppressed due to low confidence (4)

packages/placeholder-plain/src/getPageRef.js:10

• Use lowercase {number} in JSDoc for consistency with other annotations.

* @param {Number} [pageNumber = 0] Desired page number

packages/placeholder-plain/src/getPageRef.js:22

• [nitpick] Enhance this error message by including the total page count (e.g., Page ${pageNumber} out of ${pagesSplit.length} pages) to aid debugging.

throw new SignPdfError(`Failed to get reference of page "${pageNumber}".`, SignPdfError.TYPE_INPUT);

packages/placeholder-plain/src/getPageRef.js:20

• Consider splitting on whitespace with split(/\s+/) instead of a single space to avoid empty tokens if there are multiple spaces or line breaks.

pages.trim().split(' ').forEach((v, i) => (i % 3 === 0 ? pagesSplit.push([v]) : pagesSplit[pagesSplit.length - 1].push(v)));

packages/placeholder-plain/src/getPageRef.js:19

• [nitpick] You could simplify the chunking logic by iterating over tokens in steps of 3 (e.g., for (let i = 0; i < tokens.length; i += 3) pagesSplit.push(tokens.slice(i, i + 3));) for clearer intent.

const pagesSplit = [];

[Copilot]

Pull request overview

Copilot reviewed 2 out of 8 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new widgetPage option changes behavior (allows placing the widget on non-first pages) but there’s no test covering multi-page PDFs or out-of-range page selection. Add/update tests to assert the annotation is added to the requested page (a multi-page fixture like resources/issue-158-test.pdf could work) and that invalid widgetPage values surface a SignPdfError.TYPE_INPUT.

[Copilot]

The widgetPage JSDoc says "Page number" but doesn’t specify whether it’s 0-based or 1-based. Since the implementation defaults to 0, clarify in the docstring (and/or error message) that this is a 0-based page index to avoid API confusion.

Suggested change

	* @property {number} [widgetPage] Page number where the widget should be placed
	* @property {number} [widgetPage] 0-based page index where the widget should be placed

[Copilot]

The /Kids parsing uses pages.trim().split(' '), which only splits on a single space and will break on valid PDFs that use other whitespace (newlines, tabs) or multiple spaces between tokens. This can mis-group the obj gen R triples and return the wrong page ref. Consider tokenizing with a whitespace regex (and filtering empty tokens) and then selecting the 3 tokens for pageNumber (or iterating until the desired index) with a clear length check.

Suggested change

	const pagesSplit = [];
	pages.trim().split(' ').forEach((v, i) => (i % 3 === 0 ? pagesSplit.push([v]) : pagesSplit[pagesSplit.length - 1].push(v)));
	const tokens = pages.trim().split(/\s+/).filter(Boolean);
	const pagesSplit = [];
	for (let i = 0; i + 2 < tokens.length; i += 3) {
	pagesSplit.push(tokens.slice(i, i + 3));
	}

[Copilot]

pageNumber isn't validated to be a non-negative integer. If callers pass NaN, a float, or a string that coerces unexpectedly, the bounds check may not trigger and pagesSplit[pageNumber] can be undefined, causing a runtime TypeError on .join(...) instead of a SignPdfError. Validate/coerce upfront (e.g., Number.isInteger) and throw SignPdfError.TYPE_INPUT on invalid values.