From a163deb6ebbc903a0ecbc1ec48ecdd37f4da1480 Mon Sep 17 00:00:00 2001 From: Jayesh Bhade <52350067+Jaybhade@users.noreply.github.com> Date: Sat, 8 Aug 2026 00:21:33 +0530 Subject: [PATCH 1/6] Fix the two MathClamp calls the prefer-math-clamp fixer mis-ordered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule rewrites `Math.max(C, Math.min(A, B))` to `MathClamp(A, C, B)`, always taking the inner call's first operand as the value. `Math.min` is commutative, so the value is just as often the second one, and then the fixer swaps it with the upper bound: the result is `Math.min(C, v)`, which no longer applies the lower bound at all. Both call sites the fixer rewrote hit that case, so the max-outer patterns are now reported without a fix. Reporting them still needs its own message. `Math.max(C, Math.min(A, B))` is only `MathClamp(A, C, B)` when `C <= B`, and the rule can't know whether that holds, so telling the reader to "use MathClamp" is wrong advice. The message states the condition instead, and `useClamp` is left to the min-outer patterns, which are exact. In `PSStackBasedInterpreter.build` the outputs were therefore never clamped up to the Range minimum. The interpreter runs whenever `PSStackToTree` can't turn the program into a tree — a stack-shrinking `if`, or a `copy`/`index`/`roll` whose operand isn't a constant — and it then let a Type 4 function return e.g. -1.5 for a component declared as `/Range [0 1]`. In `SplitView.#clampFirstSize` the first pane's size was never clamped up to `#minSize`: dragging the resizer past it wrote a negative `flexGrow`. The existing range-clamping test only checked the upper bound, which is the one the wrong operand order preserves. --- external/eslint_plugins/prefer-math-clamp.mjs | 44 +++++++++---------- src/core/postscript/js_evaluator.js | 2 +- test/unit/postscript_spec.js | 6 +++ web/internal/split_view.js | 4 +- 4 files changed, 29 insertions(+), 27 deletions(-) diff --git a/external/eslint_plugins/prefer-math-clamp.mjs b/external/eslint_plugins/prefer-math-clamp.mjs index 989de21dd11dc..4a2857f27fa70 100644 --- a/external/eslint_plugins/prefer-math-clamp.mjs +++ b/external/eslint_plugins/prefer-math-clamp.mjs @@ -5,8 +5,16 @@ * Detected patterns and their fixes: * Math.min(Math.max(A, B), C) → MathClamp(A, B, C) * Math.min(C, Math.max(A, B)) → MathClamp(A, B, C) - * Math.max(Math.min(A, B), C) → MathClamp(A, C, B) - * Math.max(C, Math.min(A, B)) → MathClamp(A, C, B) + * Math.max(Math.min(A, B), C) → flagged as a possible clamp, not fixed + * Math.max(C, Math.min(A, B)) → flagged as a possible clamp, not fixed + * + * The min-outer patterns are exact: `Math.min(Math.max(A, B), C)` is what + * `MathClamp(A, B, C)` expands to, whichever inner operand is the value. + * + * The max-outer ones are not. `Math.min` is commutative, so the value can be A + * or B, and `MathClamp(A, C, B)` only agrees with `Math.max(C, Math.min(A, B))` + * when C <= B. The rule can't tell, so it reports these with a message saying + * as much and leaves the rewrite to a human. */ function isMathCall(node, method) { @@ -39,6 +47,9 @@ const preferMathClampRule = { messages: { useClamp: "Use MathClamp(v, min, max) instead of nested Math.min/Math.max.", + maybeClamp: + "Math.max(C, Math.min(A, B)) is MathClamp(A, C, B) only when C <= B; " + + "rewrite it by hand if that holds.", }, schema: [], }, @@ -79,32 +90,17 @@ const preferMathClampRule = { } // Pattern: Math.max(Math.min(A, B), C) or Math.max(C, Math.min(A, B)). - // Fix as MathClamp(A, C, B) where A,B are inner args, C is outer arg. + // Only a clamp when the bounds are ordered, so this is reported as a + // candidate and not fixed, see the file header. if (isMathCall(node, "max")) { const [arg0, arg1] = node.arguments; - let outerArg, innerNode; - if (isMathCall(arg0, "min") && !isMathMinMax(arg1)) { - innerNode = arg0; - outerArg = arg1; - } else if (isMathCall(arg1, "min") && !isMathMinMax(arg0)) { - innerNode = arg1; - outerArg = arg0; - } else { - return; + if ( + (isMathCall(arg0, "min") && !isMathMinMax(arg1)) || + (isMathCall(arg1, "min") && !isMathMinMax(arg0)) + ) { + context.report({ node, messageId: "maybeClamp" }); } - - const v = src.getText(innerNode.arguments[0]); - const max = src.getText(innerNode.arguments[1]); - const min = src.getText(outerArg); - - context.report({ - node, - messageId: "useClamp", - fix(fixer) { - return fixer.replaceText(node, `MathClamp(${v}, ${min}, ${max})`); - }, - }); } }, }; diff --git a/src/core/postscript/js_evaluator.js b/src/core/postscript/js_evaluator.js index c6e825402fc37..02746bf1a3328 100644 --- a/src/core/postscript/js_evaluator.js +++ b/src/core/postscript/js_evaluator.js @@ -781,7 +781,7 @@ class PSStackBasedInterpreter { const base = this.#sp - nOut; for (let i = 0; i < nOut; i++) { const v = base + i >= 0 ? this.#stack[base + i] : 0; - dest[destOffset + i] = MathClamp(range[i * 2 + 1], range[i * 2], v); + dest[destOffset + i] = MathClamp(v, range[i * 2], range[i * 2 + 1]); } }; } diff --git a/test/unit/postscript_spec.js b/test/unit/postscript_spec.js index 1e4e131ba69fc..c270e248cc585 100644 --- a/test/unit/postscript_spec.js +++ b/test/unit/postscript_spec.js @@ -733,6 +733,12 @@ describe("PostScript Type 4 lexer, parser, and Wasm compiler", function () { expect(r).toBeCloseTo(0.5, 9); }); + it("clamps output to the bottom of the declared range", async function () { + // sub falls below range [0, 1] → result clamped + const r = compileAndRun("{ sub }", [0, 1, 0, 1], [0, 1], [0.25, 0.75]); + expect(r).toBeCloseTo(0, 9); + }); + // Bitwise. it("compiles bitshift left (literal shift)", async function () { diff --git a/web/internal/split_view.js b/web/internal/split_view.js index 6a93b2c174241..c59f4f0d5fc06 100644 --- a/web/internal/split_view.js +++ b/web/internal/split_view.js @@ -107,9 +107,9 @@ class SplitView { return 0; } if (total <= this.#minSize * 2) { - return MathClamp(0, requestedFirst, total); + return MathClamp(requestedFirst, 0, total); } - return MathClamp(total - this.#minSize, this.#minSize, requestedFirst); + return MathClamp(requestedFirst, this.#minSize, total - this.#minSize); } #resize(newFirst) { From 7d71ebda472373c53dec49d86260d18ec3df86ac Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 12 Aug 2026 09:58:55 +0200 Subject: [PATCH 2/6] Let the `StructElementNode.prototype.tableAttributes` getter return a Map Using a Map seems more appropriate given that there's a bunch of adding/removing of entries, and it's especially helpful at the end of the getter since it's easy/efficient to determine its size (whereas an Object requires iterating though it to do that). --- src/core/struct_tree.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/core/struct_tree.js b/src/core/struct_tree.js index 3bfa19fb6073e..480b5ca58aa96 100644 --- a/src/core/struct_tree.js +++ b/src/core/struct_tree.js @@ -691,7 +691,7 @@ class StructElementNode { return null; } - const result = Object.create(null); + const map = new Map(); for (const attributes of this.attributes) { if (!isName(attributes.get("O"), "Table")) { continue; @@ -701,9 +701,9 @@ class StructElementNode { if (attributes.has("Summary")) { const summary = attributes.get("Summary"); if (typeof summary === "string" && summary) { - result.summary = stringToPDFString(summary); + map.set("summary", stringToPDFString(summary)); } else { - delete result.summary; + map.delete("summary"); } } continue; @@ -715,46 +715,46 @@ class StructElementNode { } const value = attributes.get(key); if (Number.isInteger(value) && value > 1) { - result[name] = value; + map.set(name, value); } else { // Omit default and invalid values, clearing any earlier class value. - delete result[name]; + map.delete(name); } } if (attributes.has("Headers")) { - delete result.headers; + map.delete("headers"); const headers = attributes.getArray("Headers"); if (Array.isArray(headers)) { const ids = headers .filter(header => typeof header === "string") .map(header => stringToPDFString(header)); if (ids.length > 0) { - result.headers = ids; + map.set("headers", ids); } } } if (role === "TH" && attributes.has("Scope")) { - delete result.scope; + map.delete("scope"); const scope = attributes.get("Scope"); if ( scope instanceof Name && ["Row", "Column", "Both"].includes(scope.name) ) { - result.scope = scope.name; + map.set("scope", scope.name); } } if (role === "TH" && attributes.has("Short")) { - delete result.short; + map.delete("short"); const short = attributes.get("Short"); if (typeof short === "string" && short) { - result.short = stringToPDFString(short); + map.set("short", stringToPDFString(short)); } } } - return Object.keys(result).length > 0 ? result : null; + return map.size ? map : null; } parseKids() { @@ -1044,10 +1044,10 @@ class StructTreePage { if (obj.role === "TH" && typeof structId === "string" && structId) { obj.structId = stringToPDFString(structId); } - const tableAttributes = node.tableAttributes; - if (tableAttributes) { - Object.assign(obj, tableAttributes); - } + node.tableAttributes?.forEach((val, key) => { + obj[key] = val; + }); + if (obj.role === "Formula") { try { const { mathML } = node; From 4b0ba029e3ec863e9b5531ca357e722087e9aef7 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 12 Aug 2026 12:38:29 +0200 Subject: [PATCH 3/6] Simplify the `Stepper.prototype.getNextBreakPoint` method --- web/debugger.mjs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/web/debugger.mjs b/web/debugger.mjs index 16ebd141e4256..cbaaf790dcf4f 100644 --- a/web/debugger.mjs +++ b/web/debugger.mjs @@ -624,12 +624,7 @@ class Stepper { getNextBreakPoint() { this.breakPoints.sort((a, b) => a - b); - for (const breakPoint of this.breakPoints) { - if (breakPoint > this.currentIdx) { - return breakPoint; - } - } - return null; + return this.breakPoints.find(idx => idx > this.currentIdx) ?? null; } breakIt(idx, callback) { From efb912aa5825606f0d0b22211bc9761ad6cbac43 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 13 Aug 2026 09:43:24 +0200 Subject: [PATCH 4/6] Only define the `getRawData` API methods when building INTERNAL_VIEWER Given that the relevant worker-thread message handler is conditionally defined, note [this code](https://github.com/mozilla/pdf.js/blob/5903d58d58e4dd9ce6ffa3834aea8480f06b4ada/src/core/worker.js#L987-L1005), it makes sense to do the same thing on the main-thread as well. This way we avoid bundling a small amount of unused code in e.g. the Firefox PDF Viewer. --- src/display/api.js | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 5a49a0954b34e..52a77cfb8a9c7 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -677,6 +677,15 @@ class PDFDocumentProxy { this._pdfInfo = pdfInfo; this._transport = transport; + if ( + typeof PDFJSDev === "undefined" || + PDFJSDev.test("TESTING || INTERNAL_VIEWER") + ) { + // For the PDF debugger. + Object.defineProperty(this, "getRawData", { + value: data => this._transport.getRawData(data), + }); + } if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) { // For testing purposes. Object.defineProperty(this, "getNetworkStreamName", { @@ -1029,10 +1038,6 @@ class PDFDocumentProxy { return this._transport.downloadInfoCapability.promise; } - getRawData(data) { - return this._transport.getRawData(data); - } - /** * Cleans up resources allocated by the document on both the main and worker * threads. @@ -2458,6 +2463,15 @@ class WorkerTransport { this.setupMessageHandler(); + if ( + typeof PDFJSDev === "undefined" || + PDFJSDev.test("TESTING || INTERNAL_VIEWER") + ) { + // For the PDF debugger. + Object.defineProperty(this, "getRawData", { + value: data => this.messageHandler.sendWithPromise("GetRawData", data), + }); + } if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) { // For testing purposes. Object.defineProperty(this, "getNetworkStreamName", { @@ -3204,10 +3218,6 @@ class WorkerTransport { return this.messageHandler.sendWithPromise("GetMarkInfo", null); } - getRawData(data) { - return this.messageHandler.sendWithPromise("GetRawData", data); - } - async startCleanup(keepLoadedFonts = false) { if (this.destroyed) { return; // No need to manually clean-up when destruction has started. From ec8dd8167a04f0860489c9977d37cb4a062b736e Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 13 Aug 2026 13:04:23 +0200 Subject: [PATCH 5/6] Remove unnecessary `typeof` check in `Field.prototype.currentValueIndices` There's no point in first checking if something is a number, when we *immediately* afterwards have a `Number.isInteger(...)` check. --- src/scripting_api/field.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/scripting_api/field.js b/src/scripting_api/field.js index 28edf8eab8b05..6b24d3e5c1cdb 100644 --- a/src/scripting_api/field.js +++ b/src/scripting_api/field.js @@ -109,13 +109,7 @@ class Field extends PDFObject { indices = [indices]; } if ( - !indices.every( - i => - typeof i === "number" && - Number.isInteger(i) && - i >= 0 && - i < this.numItems - ) + !indices.every(i => Number.isInteger(i) && i >= 0 && i < this.numItems) ) { return; } From b50c4cfee2a69d3c4f97f2d9594f243db5b10ef8 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 13 Aug 2026 11:41:23 +0200 Subject: [PATCH 6/6] Avoid creating a couple of intermediate Arrays --- src/shared/util.js | 2 +- test/integration/freetext_editor_spec.mjs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/shared/util.js b/src/shared/util.js index ec9446288e8cc..d4b3e5748d42f 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -722,7 +722,7 @@ class Util { return shadow( this, "hexNums", - Array.from(Array(256).keys(), n => n.toString(16).padStart(2, "0")) + Array.from({ length: 256 }, (_, n) => n.toString(16).padStart(2, "0")) ); } diff --git a/test/integration/freetext_editor_spec.mjs b/test/integration/freetext_editor_spec.mjs index 263b622a454db..ada2868e2d29f 100644 --- a/test/integration/freetext_editor_spec.mjs +++ b/test/integration/freetext_editor_spec.mjs @@ -3729,9 +3729,7 @@ describe("FreeText Editor", () => { const { map } = window.PDFViewerApplication.pdfDocument.annotationStorage .serializable; - return ( - map.size === 4 && [...map.values()].every(entry => entry.deleted) - ); + return map.size === 4 && map.values().every(entry => entry.deleted); }); // Disable editing mode.