From 197e806c8605bf5f8e5c451cf4297e79054e20b6 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 20 Jun 2023 16:44:09 +0200 Subject: [PATCH 1/2] [api-minor] Ensure that the `AnnotationLayer` gets a default l10n-instance in GENERIC builds (PR 16552 follow-up) *This is something that I completely overlooked during review of PR 16552, despite leaving a l10n-related comment.* The new l10n-handling of PopupAnnotations assume that the `AnnotationLayer` is always initialized with a l10n-instance, which might not actually be the case in third-party implementations where the default viewer isn't used. To work-around that we'll now bundle, and fallback on, the existing `NullL10n`-implementation in GENERIC builds of the PDF.js library. This will only result in a slight file-size increase for the *built* `pdf.js` file, again limited to GENERIC builds, since the `web/l10n_utils.js` file has no dependencies. Also, tweaks a couple of TESTING pre-processor checks to *only* include that code when running the reference tests. --- src/display/annotation_layer.js | 23 +++++++++++++---------- web/l10n_utils.js | 9 +++++++++ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index 8c81be108..49f1ff95d 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -1943,14 +1943,10 @@ class PopupElement { // The modification date is shown in the popup instead of the creation // date if it is available and can be parsed correctly, which is // consistent with other viewers such as Adobe Acrobat. - this.#dateTimePromise = parent.l10n.get( - "annotation_date_string", - { - date: dateObject.toLocaleDateString(), - time: dateObject.toLocaleTimeString(), - }, - "{{date}}, {{time}}" - ); + this.#dateTimePromise = parent.l10n.get("annotation_date_string", { + date: dateObject.toLocaleDateString(), + time: dateObject.toLocaleTimeString(), + }); } this.trigger = elements.flatMap(e => e.getElementsToTriggerPopup()); @@ -1969,7 +1965,7 @@ class PopupElement { this.#toggle(); } - if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) { + if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) { // Since the popup is lazily created, we need to ensure that it'll be // created and displayed during reference tests. this.#parent.popupShow.push(async () => { @@ -2788,7 +2784,14 @@ class AnnotationLayer { this.viewport = viewport; this.zIndex = 0; - if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) { + if ( + typeof PDFJSDev !== "undefined" && + PDFJSDev.test("GENERIC && !TESTING") + ) { + const { NullL10n } = require("pdfjs-web/l10n_utils.js"); + this.l10n ||= NullL10n; + } + if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) { // For testing purposes. Object.defineProperty(this, "showPopups", { value: async () => { diff --git a/web/l10n_utils.js b/web/l10n_utils.js index 42e2d46de..c4931135b 100644 --- a/web/l10n_utils.js +++ b/web/l10n_utils.js @@ -13,6 +13,13 @@ * limitations under the License. */ +/** + * PLEASE NOTE: This file is currently imported in both the `web/` and + * `src/display/` folders, hence be EXTREMELY careful about + * introducing any dependencies here since that can lead to an + * unexpected/unnecessary size increase of the *built* files. + */ + /** * A subset of the l10n strings in the `l10n/en-US/viewer.properties` file. */ @@ -63,6 +70,8 @@ const DEFAULT_L10N_STRINGS = { unexpected_response_error: "Unexpected server response.", rendering_error: "An error occurred while rendering the page.", + annotation_date_string: "{{date}}, {{time}}", + printing_not_supported: "Warning: Printing is not fully supported by this browser.", printing_not_ready: "Warning: The PDF is not fully loaded for printing.", From 19880fcf9ac8cbbafee3aab2993aca2338f3b370 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 20 Jun 2023 16:46:51 +0200 Subject: [PATCH 2/2] [api-minor] Move the l10n-translation into the `AnnotationLayer` With the changes in PR 16552 we can now move general translation into the `AnnotationLayer` itself, which should improve things ever so slightly in third-party implementations where the default viewer isn't used. --- src/display/annotation_layer.js | 4 +++- test/driver.js | 3 +-- web/annotation_layer_builder.js | 3 +-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index 49f1ff95d..1165d1e39 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -2823,7 +2823,7 @@ class AnnotationLayer { * @param {AnnotationLayerParameters} params * @memberof AnnotationLayer */ - render(params) { + async render(params) { const { annotations } = params; const layer = this.div; setLayerDimensions(layer, this.viewport); @@ -2897,6 +2897,8 @@ class AnnotationLayer { } this.#setAnnotationCanvasMap(); + + await this.l10n.translate(layer); } /** diff --git a/test/driver.js b/test/driver.js index b2ce75bc5..c42a8bf7b 100644 --- a/test/driver.js +++ b/test/driver.js @@ -235,9 +235,8 @@ class Rasterize { l10n, viewport: annotationViewport, }); - annotationLayer.render(parameters); + await annotationLayer.render(parameters); await annotationLayer.showPopups(); - await l10n.translate(div); // Inline SVG images from text annotations. await inlineImages(div); diff --git a/web/annotation_layer_builder.js b/web/annotation_layer_builder.js index 1edcf6f94..b653a6b0d 100644 --- a/web/annotation_layer_builder.js +++ b/web/annotation_layer_builder.js @@ -134,7 +134,7 @@ class AnnotationLayerBuilder { viewport: viewport.clone({ dontFlip: true }), }); - this.annotationLayer.render({ + await this.annotationLayer.render({ annotations, imageResourcesPath: this.imageResourcesPath, renderForms: this.renderForms, @@ -145,7 +145,6 @@ class AnnotationLayerBuilder { hasJSActions, fieldObjects, }); - this.l10n.translate(div); // Ensure that interactive form elements in the annotationLayer are // disabled while PresentationMode is active (see issue 12232).