From de628cec59e65cf5b2597ef0af8f27c7c3b1f592 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 14 Nov 2020 13:38:36 +0100 Subject: [PATCH] Some `hasJSActions`, and general annotation-code, related cleanup in the viewer and API - Add support for logical assignment operators, i.e. `&&=`, `||=`, and `??=`, with a Babel-plugin. Given that these required incrementing the ECMAScript version in the ESLint and Acorn configurations, and that platform/browser support is still fairly limited, always transpiling them seems appropriate for now. - Cache the `hasJSActions` promise in the API, similar to the existing `getAnnotations` caching. With this implemented, the lookup should now be cheap enough that it can be called unconditionally in the viewer. - Slightly improve cleanup of resources when destroying the `WorkerTransport`. - Remove the `annotationStorage`-property from the `PDFPageView` constructor, since it's not necessary and also brings it more inline with the `BaseViewer`. - Update the `BaseViewer.createAnnotationLayerBuilder` method to actaually agree with the `IPDFAnnotationLayerFactory` interface.[1] - Slightly tweak a couple of JSDoc comments. --- [1] We probably ought to re-factor both the `IPDFTextLayerFactory` and `IPDFAnnotationLayerFactory` interfaces to take parameter objects instead, since especially the `IPDFAnnotationLayerFactory` one is becoming quite unwieldy. Given that that would likely be a breaking change for any custom viewer-components implementation, this probably requires careful deprecation. --- .eslintrc | 2 +- external/builder/preprocessor2.js | 2 +- gulpfile.js | 3 +++ package.json | 1 + src/display/api.js | 11 +++++++++-- web/annotation_layer_builder.js | 4 ++-- web/base_viewer.js | 18 ++++++++++-------- web/pdf_page_view.js | 8 +++----- 8 files changed, 30 insertions(+), 19 deletions(-) diff --git a/.eslintrc b/.eslintrc index 048e910ad..e06d92908 100644 --- a/.eslintrc +++ b/.eslintrc @@ -1,6 +1,6 @@ { "parserOptions": { - "ecmaVersion": 2020, + "ecmaVersion": 2021, "sourceType": "module", }, diff --git a/external/builder/preprocessor2.js b/external/builder/preprocessor2.js index 1ca9bf98f..1b1c9320e 100644 --- a/external/builder/preprocessor2.js +++ b/external/builder/preprocessor2.js @@ -8,7 +8,7 @@ var path = require("path"); var PDFJS_PREPROCESSOR_NAME = "PDFJSDev"; var ROOT_PREFIX = "$ROOT/"; -const ACORN_ECMA_VERSION = 2020; +const ACORN_ECMA_VERSION = 2021; function isLiteral(obj, value) { return obj.type === "Literal" && obj.value === value; diff --git a/gulpfile.js b/gulpfile.js index a39616c49..900b6d0a9 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -226,6 +226,7 @@ function createWebpackConfig(defines, output) { options: { presets: skipBabel ? undefined : ["@babel/preset-env"], plugins: [ + "@babel/plugin-proposal-logical-assignment-operators", "@babel/plugin-transform-modules-commonjs", [ "@babel/plugin-transform-runtime", @@ -570,6 +571,7 @@ gulp.task("default_preferences-pre", function () { sourceType: "module", presets: undefined, // SKIP_BABEL plugins: [ + "@babel/plugin-proposal-logical-assignment-operators", "@babel/plugin-transform-modules-commonjs", babelPluginReplaceNonWebPackRequire, ], @@ -1234,6 +1236,7 @@ function buildLib(defines, dir) { sourceType: "module", presets: skipBabel ? undefined : ["@babel/preset-env"], plugins: [ + "@babel/plugin-proposal-logical-assignment-operators", "@babel/plugin-transform-modules-commonjs", [ "@babel/plugin-transform-runtime", diff --git a/package.json b/package.json index dfe9ab858..8838ac5e4 100644 --- a/package.json +++ b/package.json @@ -3,6 +3,7 @@ "version": "2.0.0", "devDependencies": { "@babel/core": "^7.12.3", + "@babel/plugin-proposal-logical-assignment-operators": "^7.12.1", "@babel/plugin-transform-modules-commonjs": "^7.12.1", "@babel/plugin-transform-runtime": "^7.12.1", "@babel/preset-env": "^7.12.1", diff --git a/src/display/api.js b/src/display/api.js index a95182a7e..6482121ef 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -905,7 +905,7 @@ class PDFDocumentProxy { /** * @returns {Promise} A promise that is resolved with `true` - * if some /AcroForm fields have JavaScript actions. + * if some /AcroForm fields have JavaScript actions. */ hasJSActions() { return this._transport.hasJSActions(); @@ -2128,7 +2128,10 @@ class WorkerTransport { const terminated = this.messageHandler.sendWithPromise("Terminate", null); waitOn.push(terminated); Promise.all(waitOn).then(() => { + this.commonObjs.clear(); this.fontLoader.clear(); + this._hasJSActionsPromise = null; + if (this._networkStream) { this._networkStream.cancelAllRequests( new AbortException("Worker was terminated.") @@ -2577,7 +2580,10 @@ class WorkerTransport { } hasJSActions() { - return this.messageHandler.sendWithPromise("HasJSActions", null); + return (this._hasJSActionsPromise ||= this.messageHandler.sendWithPromise( + "HasJSActions", + null + )); } getCalculationOrderIds() { @@ -2679,6 +2685,7 @@ class WorkerTransport { } this.commonObjs.clear(); this.fontLoader.clear(); + this._hasJSActionsPromise = null; }); } diff --git a/web/annotation_layer_builder.js b/web/annotation_layer_builder.js index 41d3a39f5..194139fa8 100644 --- a/web/annotation_layer_builder.js +++ b/web/annotation_layer_builder.js @@ -137,8 +137,8 @@ class DefaultAnnotationLayerFactory { * for annotation icons. Include trailing slash. * @param {boolean} renderInteractiveForms * @param {IL10n} l10n - * @param {boolean} enableScripting - * @param {Promise} hasJSActionsPromise + * @param {boolean} [enableScripting] + * @param {Promise} [hasJSActionsPromise] * @returns {AnnotationLayerBuilder} */ createAnnotationLayerBuilder( diff --git a/web/base_viewer.js b/web/base_viewer.js index 2c343200e..3b9b78ca4 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -469,8 +469,7 @@ class BaseViewer { } const pagesCount = pdfDocument.numPages; const firstPagePromise = pdfDocument.getPage(1); - - const annotationStorage = pdfDocument.annotationStorage; + // Rendering (potentially) depends on this, hence fetching it immediately. const optionalContentConfigPromise = pdfDocument.getOptionalContentConfig(); this._pagesCapability.promise.then(() => { @@ -521,7 +520,6 @@ class BaseViewer { id: pageNum, scale, defaultViewport: viewport.clone(), - annotationStorage, optionalContentConfigPromise, renderingQueue: this.renderingQueue, textLayerFactory, @@ -1244,11 +1242,14 @@ class BaseViewer { /** * @param {HTMLDivElement} pageDiv * @param {PDFPage} pdfPage + * @param {AnnotationStorage} [annotationStorage] - Storage for annotation + * data in forms. * @param {string} [imageResourcesPath] - Path for image resources, mainly * for annotation icons. Include trailing slash. * @param {boolean} renderInteractiveForms * @param {IL10n} l10n * @param {boolean} [enableScripting] + * @param {Promise} [hasJSActionsPromise] * @returns {AnnotationLayerBuilder} */ createAnnotationLayerBuilder( @@ -1258,21 +1259,22 @@ class BaseViewer { imageResourcesPath = "", renderInteractiveForms = false, l10n = NullL10n, - enableScripting = false + enableScripting = false, + hasJSActionsPromise = null ) { return new AnnotationLayerBuilder({ pageDiv, pdfPage, - annotationStorage, + annotationStorage: + annotationStorage || this.pdfDocument?.annotationStorage, imageResourcesPath, renderInteractiveForms, linkService: this.linkService, downloadManager: this.downloadManager, l10n, enableScripting, - hasJSActionsPromise: enableScripting - ? this.pdfDocument.hasJSActions() - : Promise.resolve(false), + hasJSActionsPromise: + hasJSActionsPromise || this.pdfDocument?.hasJSActions(), }); } diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 72b338764..c32259641 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -38,8 +38,6 @@ import { viewerCompatibilityParams } from "./viewer_compatibility.js"; * @property {number} id - The page unique ID (normally its number). * @property {number} scale - The page scale display. * @property {PageViewport} defaultViewport - The page viewport. - * @property {AnnotationStorage} [annotationStorage] - Storage for annotation - * data in forms. The default value is `null`. * @property {Promise} [optionalContentConfigPromise] - * A promise that is resolved with an {@link OptionalContentConfig} instance. * The default value is `null`. @@ -89,7 +87,6 @@ class PDFPageView { this.scale = options.scale || DEFAULT_SCALE; this.viewport = defaultViewport; this.pdfPageRotate = defaultViewport.rotation; - this._annotationStorage = options.annotationStorage || null; this._optionalContentConfigPromise = options.optionalContentConfigPromise || null; this.hasRestrictedScaling = false; @@ -549,11 +546,12 @@ class PDFPageView { this.annotationLayer = this.annotationLayerFactory.createAnnotationLayerBuilder( div, pdfPage, - this._annotationStorage, + /* annotationStorage = */ null, this.imageResourcesPath, this.renderInteractiveForms, this.l10n, - this.enableScripting + this.enableScripting, + /* hasJSActionsPromise = */ null ); } this._renderAnnotationLayer();