[api-minor] Re-factor NullL10n and remove the hard-coded l10n strin…

…gs (PR 17115 follow-up) *Please note:* These changes only affect the GENERIC build, since `NullL10n` is only a stub elsewhere (see PR 17135). After the changes in PR 17115, which modernized and improved l10n-handling, the `NullL10n`-implementation is no longer a good fallback for the "proper" `L10n`-classes. To improve this situation, especially for the *standalone* viewer-components, this patch makes the following changes: - Let the `NullL10n`-implementation extend an actual `L10n`-class, which is constant and lazily initialized, to ensure that it works *exactly* like the "proper" ones. - Automatically bundle the "en-US" l10n-strings in the build, via the pre-processor, such that we don't need to remember to manually update them. - Ensure that the *standalone* viewer-components register their DOM-elements for translation, similar to the default viewer, since this will allow future code improvements by using "data-l10n-id"/"data-l10n-args" in most (if not all) parts of the viewer. - Remove the `NullL10n` from the `AnnotationLayer`, to avoid affecting bundle size too much. For third-party users that access the `AnnotationLayer`, as exposed in the main PDF.js library, they'll now need to *manually* register it for translation. (However, the *standalone* viewer-components still works given the point above.)
mozilla · Oct 20, 2023 · 3a4ea91 · 3a4ea91
1 parent 5f0e560
commit 3a4ea91
Show file tree

Hide file tree

Showing 11 changed files with 75 additions and 125 deletions.
diff --git a/gulpfile.mjs b/gulpfile.mjs
@@ -192,6 +192,7 @@ function createWebpackConfig(
     DEFAULT_PREFERENCES: defaultPreferencesDir
       ? getDefaultPreferences(defaultPreferencesDir)
       : {},
+    DEFAULT_FTL: defines.GENERIC ? getDefaultFtl() : "",
   };
   const licenseHeaderLibre = fs
     .readFileSync("./src/license_header_libre.js")
@@ -246,7 +247,6 @@ function createWebpackConfig(
   };
   const libraryAlias = {
     "display-fetch_stream": "src/display/stubs.js",
-    "display-l10n_utils": "src/display/stubs.js",
     "display-network": "src/display/stubs.js",
     "display-node_stream": "src/display/stubs.js",
     "display-node_utils": "src/display/stubs.js",
@@ -280,7 +280,6 @@ function createWebpackConfig(
     // the tsconfig.json file for the type generation to work.
     // In the tsconfig.json files, the .js extension must be omitted.
     libraryAlias["display-fetch_stream"] = "src/display/fetch_stream.js";
-    libraryAlias["display-l10n_utils"] = "web/l10n_utils.js";
     libraryAlias["display-network"] = "src/display/network.js";
     libraryAlias["display-node_stream"] = "src/display/node_stream.js";
     libraryAlias["display-node_utils"] = "src/display/node_utils.js";
@@ -831,6 +830,21 @@ function getDefaultPreferences(dir) {
   return JSON.parse(str);
 }
 
+function getDefaultFtl() {
+  const content = fs.readFileSync("l10n/en-US/viewer.ftl").toString(),
+    stringBuf = [];
+
+  // Strip out comments and line-breaks.
+  const regExp = /^\s*#/;
+  for (const line of content.split("\n")) {
+    if (!line || regExp.test(line)) {
+      continue;
+    }
+    stringBuf.push(line);
+  }
+  return stringBuf.join("\n");
+}
+
 gulp.task("locale", function () {
   const VIEWER_LOCALE_OUTPUT = "web/locale/";
 
@@ -1544,7 +1558,6 @@ function buildLibHelper(bundleDefines, inputStream, outputDir) {
     map: {
       "pdfjs-lib": "../pdf.js",
       "display-fetch_stream": "./fetch_stream.js",
-      "display-l10n_utils": "../web/l10n_utils.js",
       "display-network": "./network.js",
       "display-node_stream": "./node_stream.js",
       "display-node_utils": "./node_utils.js",
@@ -1572,6 +1585,7 @@ function buildLib(defines, dir) {
     DEFAULT_PREFERENCES: getDefaultPreferences(
       defines.SKIP_BABEL ? "lib/" : "lib-legacy/"
     ),
+    DEFAULT_FTL: getDefaultFtl(),
   };
 
   const inputStream = merge([

diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js
@@ -41,7 +41,6 @@ import {
 } from "./display_utils.js";
 import { AnnotationStorage } from "./annotation_storage.js";
 import { ColorConverters } from "../shared/scripting_utils.js";
-import { NullL10n } from "display-l10n_utils";
 import { XfaLayer } from "./xfa_layer.js";
 
 const DEFAULT_TAB_INDEX = 1000;
@@ -2902,12 +2901,6 @@ class AnnotationLayer {
     this.viewport = viewport;
     this.zIndex = 0;
 
-    if (
-      typeof PDFJSDev !== "undefined" &&
-      PDFJSDev.test("GENERIC && !TESTING")
-    ) {
-      this.l10n ||= NullL10n;
-    }
     if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
       // For testing purposes.
       Object.defineProperty(this, "showPopups", {
@@ -3008,14 +3001,6 @@ class AnnotationLayer {
     }
 
     this.#setAnnotationCanvasMap();
-
-    if (
-      typeof PDFJSDev !== "undefined" &&
-      PDFJSDev.test("GENERIC && !TESTING") &&
-      this.l10n instanceof NullL10n
-    ) {
-      await this.l10n.translate(layer);
-    }
   }
 
   /**

diff --git a/src/display/stubs.js b/src/display/stubs.js
@@ -17,7 +17,6 @@ const NodeCanvasFactory = null;
 const NodeCMapReaderFactory = null;
 const NodeFilterFactory = null;
 const NodeStandardFontDataFactory = null;
-const NullL10n = null;
 const PDFFetchStream = null;
 const PDFNetworkStream = null;
 const PDFNodeStream = null;
@@ -27,7 +26,6 @@ export {
   NodeCMapReaderFactory,
   NodeFilterFactory,
   NodeStandardFontDataFactory,
-  NullL10n,
   PDFFetchStream,
   PDFNetworkStream,
   PDFNodeStream,

diff --git a/test/unit/unit_test.html b/test/unit/unit_test.html
@@ -21,7 +21,6 @@
         "cached-iterable": "../../node_modules/cached-iterable/src/index.mjs",
 
         "display-fetch_stream": "../../src/display/fetch_stream.js",
-        "display-l10n_utils": "../../src/display/stubs.js",
         "display-network": "../../src/display/network.js",
         "display-node_stream": "../../src/display/stubs.js",
         "display-node_utils": "../../src/display/stubs.js",

diff --git a/tsconfig.json b/tsconfig.json
@@ -11,7 +11,6 @@
     "paths": {
       "pdfjs-lib": ["./src/pdf"],
       "display-fetch_stream": ["./src/display/fetch_stream"],
-      "display-l10n_utils": ["./web/l10n_utils"],
       "display-network": ["./src/display/network"],
       "display-node_stream": ["./src/display/node_stream"],
       "display-node_utils": ["./src/display/node_utils"],

diff --git a/web/interfaces.js b/web/interfaces.js
@@ -192,12 +192,12 @@ class IL10n {
    * Translates text identified by the key and adds/formats data using the args
    * property bag. If the key was not found, translation falls back to the
    * fallback text.
-   * @param {string} key
+   * @param {Array | string} ids
    * @param {Object | null} [args]
    * @param {string} [fallback]
    * @returns {Promise<string>}
    */
-  async get(key, args = null, fallback) {}
+  async get(ids, args = null, fallback) {}
 
   /**
    * Translates HTML element.

diff --git a/web/l10n_utils.js b/web/l10n_utils.js
@@ -13,107 +13,49 @@
  * 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.
- */
+/** @typedef {import("./interfaces").IL10n} IL10n */
+
+import { FluentBundle, FluentResource } from "fluent-bundle";
+import { DOMLocalization } from "fluent-dom";
+import { L10n } from "./l10n.js";
 
 /**
- * A subset of the l10n strings in the `l10n/en-US/viewer.ftl` file.
+ * @implements {IL10n}
  */
-const DEFAULT_L10N_STRINGS = {
-  "pdfjs-of-pages": "of { $pagesCount }",
-  "pdfjs-page-of-pages": "({ $pageNumber } of { $pagesCount })",
-
-  "pdfjs-document-properties-kb": "{ $size-kb } KB ({ $size-b } bytes)",
-  "pdfjs-document-properties-mb": "{ $size-mb } MB ({ $size-b } bytes)",
-  "pdfjs-document-properties-date-string": "{ $date }, { $time }",
-  "pdfjs-document-properties-page-size-unit-inches": "in",
-  "pdfjs-document-properties-page-size-unit-millimeters": "mm",
-  "pdfjs-document-properties-page-size-orientation-portrait": "portrait",
-  "pdfjs-document-properties-page-size-orientation-landscape": "landscape",
-  "pdfjs-document-properties-page-size-name-a3": "A3",
-  "pdfjs-document-properties-page-size-name-a4": "A4",
-  "pdfjs-document-properties-page-size-name-letter": "Letter",
-  "pdfjs-document-properties-page-size-name-legal": "Legal",
-  "pdfjs-document-properties-page-size-dimension-string":
-    "{ $width } × { $height } { $unit } ({ $orientation })",
-  "pdfjs-document-properties-page-size-dimension-name-string":
-    "{ $width } × { $height } { $unit } ({ $name }, { $orientation })",
-  "pdfjs-document-properties-linearized-yes": "Yes",
-  "pdfjs-document-properties-linearized-no": "No",
-
-  "pdfjs-additional-layers": "Additional Layers",
-  "pdfjs-page-landmark": "Page { $page }",
-  "pdfjs-thumb-page-title": "Page { $page }",
-  "pdfjs-thumb-page-canvas": "Thumbnail of Page { $page }",
-
-  "pdfjs-find-reached-top": "Reached top of document, continued from bottom",
-  "pdfjs-find-reached-bottom": "Reached end of document, continued from top",
-  "pdfjs-find-match-count[one]": "{ $current } of { $total } match",
-  "pdfjs-find-match-count[other]": "{ $current } of { $total } matches",
-  "pdfjs-find-match-count-limit[one]": "More than { $limit } match",
-  "pdfjs-find-match-count-limit[other]": "More than { $limit } matches",
-  "pdfjs-find-not-found": "Phrase not found",
-
-  "pdfjs-page-scale-percent": "{ $scale }%",
-
-  "pdfjs-loading-error": "An error occurred while loading the PDF.",
-  "pdfjs-invalid-file-error": "Invalid or corrupted PDF file.",
-  "pdfjs-missing-file-error": "Missing PDF file.",
-  "pdfjs-unexpected-response-error": "Unexpected server response.",
-  "pdfjs-rendering-error": "An error occurred while rendering the page.",
-
-  "pdfjs-annotation-date-string": "{ $date }, { $time }",
-
-  "pdfjs-printing-not-supported":
-    "Warning: Printing is not fully supported by this browser.",
-  "pdfjs-printing-not-ready":
-    "Warning: The PDF is not fully loaded for printing.",
-  "pdfjs-web-fonts-disabled":
-    "Web fonts are disabled: unable to use embedded PDF fonts.",
-
-  "pdfjs-free-text-default-content": "Start typing…",
-  "pdfjs-editor-alt-text-button-label": "Alt text",
-  "pdfjs-editor-alt-text-edit-button-label": "Edit alt text",
-  "pdfjs-editor-alt-text-decorative-tooltip": "Marked as decorative",
-  "pdfjs-editor-resizer-label-top-left": "Top left corner — resize",
-  "pdfjs-editor-resizer-label-top-middle": "Top middle — resize",
-  "pdfjs-editor-resizer-label-top-right": "Top right corner — resize",
-  "pdfjs-editor-resizer-label-middle-right": "Middle right — resize",
-  "pdfjs-editor-resizer-label-bottom-right": "Bottom right corner — resize",
-  "pdfjs-editor-resizer-label-bottom-middle": "Bottom middle — resize",
-  "pdfjs-editor-resizer-label-bottom-left": "Bottom left corner — resize",
-  "pdfjs-editor-resizer-label-middle-left": "Middle left — resize",
-};
-if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
-  DEFAULT_L10N_STRINGS.print_progress_percent = "{ $progress }%";
-}
+class ConstL10n extends L10n {
+  static #instance;
+
+  constructor(lang) {
+    super({ lang });
+    this.setL10n(
+      new DOMLocalization(
+        [],
+        ConstL10n.#generateBundles.bind(ConstL10n, this.getLanguage())
+      )
+    );
+  }
 
-function getL10nFallback(key, args) {
-  switch (key) {
-    case "pdfjs-find-match-count":
-      key = `pdfjs-find-match-count[${args.total === 1 ? "one" : "other"}]`;
-      break;
-    case "pdfjs-find-match-count-limit":
-      key = `pdfjs-find-match-count-limit[${
-        args.limit === 1 ? "one" : "other"
-      }]`;
-      break;
+  static async *#generateBundles(lang) {
+    let text;
+    if (typeof PDFJSDev === "undefined") {
+      const url = new URL(`./locale/${lang}/viewer.ftl`, window.location.href);
+      const data = await fetch(url);
+      text = await data.text();
+    } else {
+      text = PDFJSDev.eval("DEFAULT_FTL");
+    }
+    const resource = new FluentResource(text);
+    const bundle = new FluentBundle(lang);
+    const errors = bundle.addResource(resource);
+    if (errors.length) {
+      console.error("L10n errors", errors);
+    }
+    yield bundle;
   }
-  return DEFAULT_L10N_STRINGS[key] || "";
-}
 
-// Replaces { $arguments } with their values.
-function formatL10nValue(text, args) {
-  if (!args) {
-    return text;
+  static get instance() {
+    return (this.#instance ||= new ConstL10n("en-US"));
   }
-  return text.replaceAll(/\{\s*$(\w+)\s*\}/g, (all, name) => {
-    return name in args ? args[name] : "{$" + name + "}";
-  });
 }
 
 /**
@@ -122,18 +64,20 @@ function formatL10nValue(text, args) {
  */
 const NullL10n = {
   getLanguage() {
-    return "en-us";
+    return ConstL10n.instance.getLanguage();
   },
 
   getDirection() {
-    return "ltr";
+    return ConstL10n.instance.getDirection();
   },
 
-  async get(key, args = null, fallback = getL10nFallback(key, args)) {
-    return formatL10nValue(fallback, args);
+  async get(ids, args = null, fallback) {
+    return ConstL10n.instance.get(ids, args, fallback);
   },
 
-  async translate(element) {},
+  async translate(element) {
+    return ConstL10n.instance.translate(element);
+  },
 };
 
 export { NullL10n };
diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js
@@ -215,6 +215,11 @@ class PDFPageView {
             optionalContentConfig.hasInitialVisibility;
         });
       }
+
+      // Ensure that Fluent is connected in e.g. the COMPONENTS build.
+      if (this.l10n === NullL10n) {
+        this.l10n.translate(this.div);
+      }
     }
   }
 

diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js
@@ -322,6 +322,14 @@ class PDFViewer {
         pdfPage?.cleanup();
       }
     });
+
+    if (
+      (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
+      this.l10n === NullL10n
+    ) {
+      // Ensure that Fluent is connected in e.g. the COMPONENTS build.
+      this.l10n.translate(this.container);
+    }
   }
 
   get pagesCount() {

diff --git a/web/viewer-geckoview.html b/web/viewer-geckoview.html
@@ -54,7 +54,6 @@
           "cached-iterable": "../node_modules/cached-iterable/src/index.mjs",
 
           "display-fetch_stream": "../src/display/fetch_stream.js",
-          "display-l10n_utils": "../src/display/stubs.js",
           "display-network": "../src/display/network.js",
           "display-node_stream": "../src/display/stubs.js",
           "display-node_utils": "../src/display/stubs.js",

diff --git a/web/viewer.html b/web/viewer.html
@@ -63,7 +63,6 @@
           "cached-iterable": "../node_modules/cached-iterable/src/index.mjs",
 
           "display-fetch_stream": "../src/display/fetch_stream.js",
-          "display-l10n_utils": "../src/display/stubs.js",
           "display-network": "../src/display/network.js",
           "display-node_stream": "../src/display/stubs.js",
           "display-node_utils": "../src/display/stubs.js",