Skip to content

Restore mobile PDF preview compatibility for browsers without Array.prototype.at #729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
klboke merged 4 commits into from
Apr 9, 2026
+80 −1
Merged

Restore mobile PDF preview compatibility for browsers without Array.prototype.at #729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
405a12e
Initial plan
Copilot Apr 9, 2026
5499150
fix: add pdfjs at compatibility polyfill
Copilot Apr 9, 2026
530304b
test: use junit assertions for pdf viewer compatibility
Copilot Apr 9, 2026
3fbf015
fix: refine pdfjs at compatibility polyfill
Copilot Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion server/src/main/resources/static/pdfjs/build/pdf.worker.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
* pdfjsVersion = 5.4.530
* pdfjsBuild = 50cc4adac
*/
import "../web/compatibility.mjs";
/******/ var __webpack_modules__ = ({

/***/ 34:
Expand Down Expand Up @@ -66999,4 +67000,4 @@ globalThis.pdfjsWorker = {

export { WorkerMessageHandler };

//# sourceMappingURL=pdf.worker.mjs.map
//# sourceMappingURL=pdf.worker.mjs.map
42 changes: 42 additions & 0 deletions server/src/main/resources/static/pdfjs/web/compatibility.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
const atTargets = [
Array,
String,
Int8Array,
Comment on lines +1 to +4
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The String polyfill uses this[resolvedIndex], which returns UTF-16 code units; native String.prototype.at is code-point aware (handles surrogate pairs). Since String.prototype.at is included in atTargets, either implement the code-point semantics or drop String from the targets to avoid introducing subtly incorrect behavior.

Copilot uses AI. Check for mistakes.
Uint8Array,
Uint8ClampedArray,
Int16Array,
Uint16Array,
Int32Array,
Uint32Array,
Float32Array,
Float64Array,
globalThis.BigInt64Array,
globalThis.BigUint64Array,
];
Comment on lines +11 to +15
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compatibility.mjs references globalThis directly when building atTargets. In environments without native globalThis (which can overlap with browsers missing Array.prototype.at), this will throw before the polyfill installs and prevent PDF.js from booting. Use a safe global-object resolution (e.g., prefer globalThis when defined, otherwise self/window) before accessing BigInt typed array constructors.

Copilot uses AI. Check for mistakes.

function installAtPolyfill(target) {
if (!target?.prototype || typeof target.prototype.at === "function") {
return;
}
Object.defineProperty(target.prototype, "at", {
value(index) {
const length = this.length >>> 0;
let relativeIndex = Number(index);
if (Number.isNaN(relativeIndex)) {
relativeIndex = 0;
}
relativeIndex = Math.trunc(relativeIndex);
Comment on lines +25 to +28
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The polyfill implementation depends on Number.isNaN and Math.trunc, but the compatibility module runs before PDF.js (and its bundled polyfills) load. On older engines these methods may be missing, causing the shim itself to crash. Consider avoiding these dependencies (e.g., NaN check via x !== x and integer conversion via Math.floor/Math.ceil) so the shim only requires the minimum baseline needed to run module scripts.

Suggested change
if (Number.isNaN(relativeIndex)) {
relativeIndex = 0;
}
relativeIndex = Math.trunc(relativeIndex);
if (relativeIndex !== relativeIndex) {
relativeIndex = 0;
}
relativeIndex =
relativeIndex < 0 ? Math.ceil(relativeIndex) : Math.floor(relativeIndex);

Copilot uses AI. Check for mistakes.
const resolvedIndex = relativeIndex >= 0 ? relativeIndex : length + relativeIndex;
if (resolvedIndex < 0 || resolvedIndex >= length) {
return undefined;
}
return this[resolvedIndex];
},
writable: true,
configurable: true,
});
}

for (const target of atTargets) {
installAtPolyfill(target);
}
1 change: 1 addition & 0 deletions server/src/main/resources/static/pdfjs/web/viewer.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

<!-- This snippet is used in production (included from viewer.html) -->
<link rel="resource" type="application/l10n" href="locale/locale.json" />
<script src="compatibility.mjs" type="module"></script>
<script src="../build/pdf.mjs" type="module"></script>

<link rel="stylesheet" href="viewer.css" />
Expand Down
35 changes: 35 additions & 0 deletions server/src/test/java/cn/keking/PdfViewerCompatibilityTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package cn.keking;

import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;

import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class PdfViewerCompatibilityTests {

@Test
void shouldLoadCompatibilityModuleBeforePdfJs() throws IOException {
String viewerHtml = readResource("/static/pdfjs/web/viewer.html");

assertTrue(viewerHtml.contains("<script src=\"compatibility.mjs\" type=\"module\"></script>"));
assertTrue(viewerHtml.indexOf("compatibility.mjs") < viewerHtml.indexOf("../build/pdf.mjs"));
}
Comment on lines +15 to +20
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These assertions are very brittle because they depend on exact HTML/script formatting (attribute order, whitespace, self-closing style). A small upstream PDF.js update could break the test without changing behavior. Consider making the check more resilient (e.g., regex for a module script whose src ends with compatibility.mjs, and order based on match positions) while still ensuring the shim loads before pdf.mjs.

Copilot uses AI. Check for mistakes.

@Test
void shouldLoadCompatibilityModuleInPdfWorker() throws IOException {
String workerScript = readResource("/static/pdfjs/build/pdf.worker.mjs");

assertTrue(workerScript.contains("import \"../web/compatibility.mjs\";"));
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion requires the worker import line to match exactly (including quote style and semicolon). To reduce false negatives on future PDF.js asset updates, consider loosening it to a substring/regex that tolerates whitespace/quotes while still verifying the compatibility module is imported from the worker entrypoint.

Suggested change
assertTrue(workerScript.contains("import \"../web/compatibility.mjs\";"));
assertTrue(workerScript.matches("(?s).*import\\s+[\"']\\.\\./web/compatibility\\.mjs[\"']\\s*;?.*"));

Copilot uses AI. Check for mistakes.
}

private String readResource(String resourcePath) throws IOException {
try (InputStream inputStream = getClass().getResourceAsStream(resourcePath)) {
assertNotNull(inputStream);
return new String(inputStream.readAllBytes(), StandardCharsets.UTF_8);
}
}
}
Loading