From 65e71714b8301f97d7583944aa0a4e4539475ce5 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Fri, 18 Mar 2022 10:27:29 +0000 Subject: [PATCH] Upgrade pdf.js via split test framework (#7053) GitOrigin-RevId: dffe1f5bec07cba0914e3bd18ff5332dfe204f33 --- package-lock.json | 78 ++++++++++++++++--- .../src/Features/Project/ProjectController.js | 17 ++++ .../web/app/views/project/editor/meta.pug | 1 + .../pdf-preview/components/pdf-js-viewer.js | 14 +++- .../features/pdf-preview/util/highlights.js | 8 +- .../pdf-preview/util/pdf-js-versions.js | 60 ++++++++++++++ .../pdf-preview/util/pdf-js-wrapper.js | 36 ++++----- services/web/package.json | 3 +- services/web/webpack.config.js | 24 ++++-- 9 files changed, 194 insertions(+), 47 deletions(-) create mode 100644 services/web/frontend/js/features/pdf-preview/util/pdf-js-versions.js diff --git a/package-lock.json b/package-lock.json index eddd8181b0..af2a90e9e9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30642,6 +30642,32 @@ "worker-loader": "^3.0.7" } }, + "node_modules/pdfjs-dist210": { + "name": "pdfjs-dist", + "version": "2.10.377", + "resolved": "https://registry.npmjs.org/pdfjs-dist/-/pdfjs-dist-2.10.377.tgz", + "integrity": "sha512-i0jRShtvgfsVQUNCoFYH4SVhPO3U0yhtiFLfZ0RR0B+68N+Vnwq+8B3cjWjLEwWGh8wg1XQ/sYMYKUlHn/Qpsw==", + "peerDependencies": { + "worker-loader": "^3.0.7" + } + }, + "node_modules/pdfjs-dist213": { + "name": "pdfjs-dist", + "version": "2.13.216", + "resolved": "https://registry.npmjs.org/pdfjs-dist/-/pdfjs-dist-2.13.216.tgz", + "integrity": "sha512-qn/9a/3IHIKZarTK6ajeeFXBkG15Lg1Fx99PxU09PAU2i874X8mTcHJYyDJxu7WDfNhV6hM7bRQBZU384anoqQ==", + "dependencies": { + "web-streams-polyfill": "^3.2.0" + }, + "peerDependencies": { + "worker-loader": "^3.0.8" + }, + "peerDependenciesMeta": { + "worker-loader": { + "optional": true + } + } + }, "node_modules/pend": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/pend/-/pend-1.2.0.tgz", @@ -42761,6 +42787,26 @@ "errno": "~0.1.7" } }, + "node_modules/worker-loader": { + "version": "3.0.8", + "resolved": "https://registry.npmjs.org/worker-loader/-/worker-loader-3.0.8.tgz", + "integrity": "sha512-XQyQkIFeRVC7f7uRhFdNMe/iJOdO6zxAaR3EWbDp45v3mDhrTi+++oswKNxShUNjPC/1xUp5DB29YKLhFo129g==", + "peer": true, + "dependencies": { + "loader-utils": "^2.0.0", + "schema-utils": "^3.0.0" + }, + "engines": { + "node": ">= 10.13.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/webpack" + }, + "peerDependencies": { + "webpack": "^4.0.0 || ^5.0.0" + } + }, "node_modules/worker-rpc": { "version": "0.1.1", "resolved": "https://registry.npmjs.org/worker-rpc/-/worker-rpc-0.1.1.tgz", @@ -46556,7 +46602,8 @@ "passport-orcid": "0.0.4", "passport-saml": "https://github.com/overleaf/passport-saml/releases/download/v3.0.0-overleaf/passport-saml-3.0.0-overleaf.tar.gz", "passport-twitter": "^1.0.4", - "pdfjs-dist": "~2.10.377", + "pdfjs-dist210": "npm:pdfjs-dist@2.10.377", + "pdfjs-dist213": "npm:pdfjs-dist@2.13.216", "prop-types": "^15.7.2", "pug": "^3.0.1", "pug-runtime": "^3.0.1", @@ -47691,14 +47738,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "services/web/node_modules/pdfjs-dist": { - "version": "2.10.377", - "resolved": "https://registry.npmjs.org/pdfjs-dist/-/pdfjs-dist-2.10.377.tgz", - "integrity": "sha512-i0jRShtvgfsVQUNCoFYH4SVhPO3U0yhtiFLfZ0RR0B+68N+Vnwq+8B3cjWjLEwWGh8wg1XQ/sYMYKUlHn/Qpsw==", - "peerDependencies": { - "worker-loader": "^3.0.7" - } - }, "services/web/node_modules/propagate": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/propagate/-/propagate-2.0.1.tgz", @@ -53999,7 +54038,8 @@ "passport-orcid": "0.0.4", "passport-saml": "https://github.com/overleaf/passport-saml/releases/download/v3.0.0-overleaf/passport-saml-3.0.0-overleaf.tar.gz", "passport-twitter": "^1.0.4", - "pdfjs-dist": "~2.10.377", + "pdfjs-dist210": "npm:pdfjs-dist@2.10.377", + "pdfjs-dist213": "npm:pdfjs-dist@2.13.216", "pirates": "^4.0.1", "postcss-loader": "^3.0.0", "prettier": "2.5.1", @@ -76022,6 +76062,19 @@ "integrity": "sha512-/ZkA1FwkEOyDaq11JhMLazdwQAA0F9uwrP7h/1L9Akt9KWh1G5/tkzS+bPuUELq2s2GDFnaT+kooN/aSjT7DXQ==", "requires": {} }, + "pdfjs-dist210": { + "version": "npm:pdfjs-dist@2.10.377", + "resolved": "https://registry.npmjs.org/pdfjs-dist/-/pdfjs-dist-2.10.377.tgz", + "integrity": "sha512-i0jRShtvgfsVQUNCoFYH4SVhPO3U0yhtiFLfZ0RR0B+68N+Vnwq+8B3cjWjLEwWGh8wg1XQ/sYMYKUlHn/Qpsw==" + }, + "pdfjs-dist213": { + "version": "npm:pdfjs-dist@2.13.216", + "resolved": "https://registry.npmjs.org/pdfjs-dist/-/pdfjs-dist-2.13.216.tgz", + "integrity": "sha512-qn/9a/3IHIKZarTK6ajeeFXBkG15Lg1Fx99PxU09PAU2i874X8mTcHJYyDJxu7WDfNhV6hM7bRQBZU384anoqQ==", + "requires": { + "web-streams-polyfill": "^3.2.0" + } + }, "pend": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/pend/-/pend-1.2.0.tgz", @@ -84466,6 +84519,11 @@ "integrity": "sha512-3ux37gEX670UUphBF9AMCq8XM6iQ8Ac6A+DSRRjDoRBm1ufCkaCDdNVbaqq60PsEkdNlLKrGtv/YBP4EJXqNtQ==", "dev": true }, + "web-streams-polyfill": { + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/web-streams-polyfill/-/web-streams-polyfill-3.2.0.tgz", + "integrity": "sha512-EqPmREeOzttaLRm5HS7io98goBgZ7IVz79aDvqjD0kYXLtFZTc0T/U6wHTPKyIjb+MdN7DFIIX6hgdBEpWmfPA==" + }, "webidl-conversions": { "version": "6.1.0", "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-6.1.0.tgz", diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 903d96b328..742d3fe424 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -814,6 +814,21 @@ const ProjectController = { } ) }, + pdfjsAssignment(cb) { + SplitTestHandler.getAssignment( + req, + 'pdfjs', + {}, + (error, assignment) => { + // do not fail editor load if assignment fails + if (error) { + cb(null, { variant: 'default' }) + } else { + cb(null, assignment) + } + } + ) + }, }, ( err, @@ -826,6 +841,7 @@ const ProjectController = { brandVariation, newSourceEditorAssignment, pdfDetachAssignment, + pdfjsAssignment, } ) => { if (err != null) { @@ -985,6 +1001,7 @@ const ProjectController = { gitBridgePublicBaseUrl: Settings.gitBridgePublicBaseUrl, wsUrl, showSupport: Features.hasFeature('support'), + pdfjsVariant: pdfjsAssignment.variant, showPdfDetach, debugPdfDetach, showNewSourceEditorOption, diff --git a/services/web/app/views/project/editor/meta.pug b/services/web/app/views/project/editor/meta.pug index 069eb73bae..b6026d4b3e 100644 --- a/services/web/app/views/project/editor/meta.pug +++ b/services/web/app/views/project/editor/meta.pug @@ -20,6 +20,7 @@ meta(name="ol-pdfImageResourcesPath" content="/images/") //- used in public/js/libs/sharejs.js meta(name="ol-useShareJsHash" data-type="boolean" content=true) meta(name="ol-wsRetryHandshake" data-type="json" content=settings.wsRetryHandshake) +meta(name="ol-pdfjsVariant" content=pdfjsVariant) meta(name="ol-showPdfDetach" data-type="boolean" content=showPdfDetach) meta(name="ol-debugPdfDetach" data-type="boolean" content=debugPdfDetach) meta(name="ol-showNewSourceEditorOption" data-type="boolean" content=showNewSourceEditorOption) diff --git a/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js b/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js index 84190b9fe5..655ffb46d7 100644 --- a/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js +++ b/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js @@ -31,9 +31,15 @@ function PdfJsViewer({ url }) { // create the viewer when the container is mounted const handleContainer = useCallback(parent => { if (parent) { - const viewer = new PDFJSWrapper(parent.firstChild) - setPdfJsWrapper(viewer) - return () => viewer.destroy() + const wrapper = new PDFJSWrapper(parent.firstChild) + wrapper.init().then(() => { + setPdfJsWrapper(wrapper) + }) + + return () => { + setPdfJsWrapper(null) + wrapper.destroy() + } } }, []) @@ -177,7 +183,7 @@ function PdfJsViewer({ url }) { for (const highlight of highlights) { try { - const element = buildHighlightElement(highlight, pdfJsWrapper.viewer) + const element = buildHighlightElement(highlight, pdfJsWrapper) elements.push(element) } catch (error) { // ignore invalid highlights diff --git a/services/web/frontend/js/features/pdf-preview/util/highlights.js b/services/web/frontend/js/features/pdf-preview/util/highlights.js index c8e0322540..e5203b8f84 100644 --- a/services/web/frontend/js/features/pdf-preview/util/highlights.js +++ b/services/web/frontend/js/features/pdf-preview/util/highlights.js @@ -1,7 +1,5 @@ -import * as PDFJS from 'pdfjs-dist/legacy/build/pdf' - -export function buildHighlightElement(highlight, viewer) { - const pageView = viewer.getPageView(highlight.page - 1) +export function buildHighlightElement(highlight, wrapper) { + const pageView = wrapper.viewer.getPageView(highlight.page - 1) const viewport = pageView.viewport @@ -14,7 +12,7 @@ export function buildHighlightElement(highlight, viewer) { height - highlight.v + 10, // yMax ]) - const [left, top, right, bottom] = PDFJS.Util.normalizeRect(rect) + const [left, top, right, bottom] = wrapper.PDFJS.Util.normalizeRect(rect) const element = document.createElement('div') element.style.left = Math.floor(left) + 'px' diff --git a/services/web/frontend/js/features/pdf-preview/util/pdf-js-versions.js b/services/web/frontend/js/features/pdf-preview/util/pdf-js-versions.js new file mode 100644 index 0000000000..9b224d6d8d --- /dev/null +++ b/services/web/frontend/js/features/pdf-preview/util/pdf-js-versions.js @@ -0,0 +1,60 @@ +// NOTE: using "legacy" build as main build requires webpack v5 +// import PDFJS from 'pdfjs-dist/webpack' + +// To add a new version, copy and adjust one of the `importPDFJS*` functions below, +// add the variant to the "switch" statement, and add to `pdfjsVersions` in webpack.config.js + +import 'core-js/features/promise/all-settled' // polyfill for Promise.allSettled (used by pdf.js) +import getMeta from '../../../utils/meta' + +async function importPDFJS210() { + const cMapUrl = '/js/pdfjs-dist210/cmaps/' + const imageResourcesPath = '/images/pdfjs-dist210' + + const [PDFJS, PDFJSViewer, { default: PDFJSWorker }] = await Promise.all([ + import('pdfjs-dist210/legacy/build/pdf'), + import('pdfjs-dist210/legacy/web/pdf_viewer'), + import('pdfjs-dist210/legacy/build/pdf.worker'), + import('pdfjs-dist210/legacy/web/pdf_viewer.css'), + ]) + + if (typeof window !== 'undefined' && 'Worker' in window) { + PDFJS.GlobalWorkerOptions.workerPort = new PDFJSWorker() + } + + return { PDFJS, PDFJSViewer, PDFJSWorker, cMapUrl, imageResourcesPath } +} + +async function importPDFJS213() { + const cMapUrl = '/js/pdfjs-dist213/cmaps/' + const imageResourcesPath = '/images/pdfjs-dist213' + + const [PDFJS, PDFJSViewer, { default: PDFJSWorker }] = await Promise.all([ + import('pdfjs-dist213/legacy/build/pdf'), + import('pdfjs-dist213/legacy/web/pdf_viewer'), + import('pdfjs-dist213/legacy/build/pdf.worker'), + import('pdfjs-dist213/legacy/web/pdf_viewer.css'), + ]) + + if (typeof window !== 'undefined' && 'Worker' in window) { + PDFJS.GlobalWorkerOptions.workerPort = new PDFJSWorker() + } + + return { PDFJS, PDFJSViewer, PDFJSWorker, cMapUrl, imageResourcesPath } +} + +async function importPDFJS() { + const variant = getMeta('ol-pdfjsVariant', 'default') + + switch (variant) { + case '213': + return importPDFJS213() + + case '210': + case 'default': + default: + return importPDFJS210() + } +} + +export default importPDFJS() diff --git a/services/web/frontend/js/features/pdf-preview/util/pdf-js-wrapper.js b/services/web/frontend/js/features/pdf-preview/util/pdf-js-wrapper.js index f9bda448de..6c91e38757 100644 --- a/services/web/frontend/js/features/pdf-preview/util/pdf-js-wrapper.js +++ b/services/web/frontend/js/features/pdf-preview/util/pdf-js-wrapper.js @@ -1,22 +1,7 @@ -// NOTE: using "legacy" build as main build requires webpack v5 -// import PDFJS from 'pdfjs-dist/webpack' - -import 'core-js/features/promise/all-settled' // polyfill for Promise.allSettled (used by pdf.js) -import * as PDFJS from 'pdfjs-dist/legacy/build/pdf' -import * as PDFJSViewer from 'pdfjs-dist/legacy/web/pdf_viewer' -import PDFJSWorker from 'pdfjs-dist/legacy/build/pdf.worker' -import 'pdfjs-dist/legacy/web/pdf_viewer.css' -import getMeta from '../../../utils/meta' import { captureMessage } from '../../../infrastructure/error-reporter' -if (typeof window !== 'undefined' && 'Worker' in window) { - PDFJS.GlobalWorkerOptions.workerPort = new PDFJSWorker() -} - const params = new URLSearchParams(window.location.search) const disableFontFace = params.get('disable-font-face') === 'true' -const cMapUrl = getMeta('ol-pdfCMapsPath') -const imageResourcesPath = getMeta('ol-pdfImageResourcesPath') const disableStream = process.env.NODE_ENV !== 'test' const rangeChunkSize = 128 * 1024 // 128K chunks @@ -24,6 +9,17 @@ const rangeChunkSize = 128 * 1024 // 128K chunks export default class PDFJSWrapper { constructor(container) { this.container = container + } + + async init() { + const { PDFJS, PDFJSViewer, PDFJSWorker, cMapUrl, imageResourcesPath } = + await import('./pdf-js-versions').then(m => m.default) + + this.PDFJS = PDFJS + this.PDFJSViewer = PDFJSViewer + this.PDFJSWorker = PDFJSWorker + this.cMapUrl = cMapUrl + this.imageResourcesPath = imageResourcesPath // create the event bus const eventBus = new PDFJSViewer.EventBus() @@ -40,7 +36,7 @@ export default class PDFJSWrapper { // create the viewer const viewer = new PDFJSViewer.PDFViewer({ - container, + container: this.container, eventBus, imageResourcesPath, linkService, @@ -66,9 +62,9 @@ export default class PDFJSWrapper { } return new Promise((resolve, reject) => { - this.loadDocumentTask = PDFJS.getDocument({ + this.loadDocumentTask = this.PDFJS.getDocument({ url, - cMapUrl, + cMapUrl: this.cMapUrl, cMapPacked: true, disableFontFace, rangeChunkSize, @@ -202,6 +198,8 @@ export default class PDFJSWrapper { this.loadDocumentTask.destroy() this.loadDocumentTask = undefined } - this.viewer.destroy() + if (this.viewer) { + this.viewer.destroy() + } } } diff --git a/services/web/package.json b/services/web/package.json index 07c68ab164..d11d992816 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -165,7 +165,8 @@ "passport-orcid": "0.0.4", "passport-saml": "https://github.com/overleaf/passport-saml/releases/download/v3.0.0-overleaf/passport-saml-3.0.0-overleaf.tar.gz", "passport-twitter": "^1.0.4", - "pdfjs-dist": "~2.10.377", + "pdfjs-dist210": "npm:pdfjs-dist@2.10.377", + "pdfjs-dist213": "npm:pdfjs-dist@2.13.216", "prop-types": "^15.7.2", "pug": "^3.0.1", "pug-runtime": "^3.0.1", diff --git a/services/web/webpack.config.js b/services/web/webpack.config.js index 575b94bce6..9a429c89a7 100644 --- a/services/web/webpack.config.js +++ b/services/web/webpack.config.js @@ -52,7 +52,8 @@ function getModuleDirectory(moduleName) { const mathjaxDir = getModuleDirectory('mathjax') const aceDir = getModuleDirectory('ace-builds') -const pdfjsDir = getModuleDirectory('pdfjs-dist') + +const pdfjsVersions = ['pdfjs-dist210', 'pdfjs-dist213'] module.exports = { // Defines the "entry point(s)" for the application - i.e. the file which @@ -318,13 +319,20 @@ module.exports = { from: `${aceDir}/src-min-noconflict`, to: `js/ace-${PackageVersions.version.ace}/`, }, - // Copy CMap files (used to provide support for non-Latin characters) - // and static images from pdfjs-dist package to build output. - { from: `${pdfjsDir}/cmaps`, to: 'js/cmaps' }, - { - from: `${pdfjsDir}/legacy/web/images`, - to: 'images', - }, + ...pdfjsVersions.flatMap(version => { + const dir = getModuleDirectory(version) + + // Copy CMap files (used to provide support for non-Latin characters) + // and static images from pdfjs-dist package to build output. + + return [ + { from: `${dir}/cmaps`, to: `js/${dir}/cmaps` }, + { + from: `${dir}/legacy/web/images`, + to: `images/${dir}`, + }, + ] + }), ]), ], }