Ensure that GenericL10n works if the locale files cannot be loaded

- Ensure that localization works in the GENERIC viewer, even if the necessary locale files cannot be loaded. This was the behaviour prior to the introduction of Fluent, and it seems worthwhile to keep that (especially since we already bundle the en-US strings anyway). - Simplify the `NullL10n`-implementation by having it extend the `L10n`-class, to reduce the maintenance burden of the code. - Stop exporting `NullL10n` in the viewer-components, since it was never intended to be used directly and only exists as a fallback. *Please note:* This doesn't affect the Firefox PDF Viewer, thanks to the use of import maps.
mozilla · Jan 30, 2024 · 12b1252 · 12b1252
1 parent 245fd02
commit 12b1252
Show file tree

Hide file tree

Showing 15 changed files with 80 additions and 142 deletions.
diff --git a/examples/mobile-viewer/viewer.mjs b/examples/mobile-viewer/viewer.mjs
@@ -272,7 +272,7 @@ const PDFViewerApplication = {
     });
     this.pdfLinkService = linkService;
 
-    this.l10n = pdfjsViewer.NullL10n;
+    this.l10n = new pdfjsViewer.GenericL10n();
 
     const container = document.getElementById("viewerContainer");
     const pdfViewer = new pdfjsViewer.PDFViewer({

diff --git a/gulpfile.mjs b/gulpfile.mjs
@@ -272,7 +272,7 @@ function createWebpackConfig(
     "web-com": "",
     "web-download_manager": "",
     "web-external_services": "",
-    "web-l10n_utils": "web/stubs.js",
+    "web-null_l10n": "",
     "web-pdf_attachment_viewer": "web/pdf_attachment_viewer.js",
     "web-pdf_cursor_tools": "web/pdf_cursor_tools.js",
     "web-pdf_document_properties": "web/pdf_document_properties.js",
@@ -294,6 +294,7 @@ function createWebpackConfig(
     viewerAlias["web-com"] = "web/chromecom.js";
     viewerAlias["web-download_manager"] = "web/download_manager.js";
     viewerAlias["web-external_services"] = "web/chromecom.js";
+    viewerAlias["web-null_l10n"] = "web/l10n.js";
     viewerAlias["web-preferences"] = "web/chromecom.js";
     viewerAlias["web-print_service"] = "web/pdf_print_service.js";
   } else if (bundleDefines.GENERIC) {
@@ -308,13 +309,12 @@ function createWebpackConfig(
     viewerAlias["web-com"] = "web/genericcom.js";
     viewerAlias["web-download_manager"] = "web/download_manager.js";
     viewerAlias["web-external_services"] = "web/genericcom.js";
-    viewerAlias["web-l10n_utils"] = "web/l10n_utils.js";
+    viewerAlias["web-null_l10n"] = "web/genericl10n.js";
     viewerAlias["web-preferences"] = "web/genericcom.js";
     viewerAlias["web-print_service"] = "web/pdf_print_service.js";
   } else if (bundleDefines.MOZCENTRAL) {
     if (bundleDefines.GECKOVIEW) {
       const gvAlias = {
-        "web-l10n_utils": "web/stubs.js",
         "web-toolbar": "web/toolbar-geckoview.js",
       };
       for (const key in viewerAlias) {
@@ -324,6 +324,7 @@ function createWebpackConfig(
     viewerAlias["web-com"] = "web/firefoxcom.js";
     viewerAlias["web-download_manager"] = "web/firefoxcom.js";
     viewerAlias["web-external_services"] = "web/firefoxcom.js";
+    viewerAlias["web-null_l10n"] = "web/l10n.js";
     viewerAlias["web-preferences"] = "web/firefoxcom.js";
     viewerAlias["web-print_service"] = "web/firefox_print_service.js";
   }
@@ -1616,7 +1617,7 @@ function buildLibHelper(bundleDefines, inputStream, outputDir) {
       "display-node_utils": "./node_utils.js",
       "fluent-bundle": "../../../node_modules/@fluent/bundle/esm/index.js",
       "fluent-dom": "../../../node_modules/@fluent/dom/esm/index.js",
-      "web-l10n_utils": "../web/l10n_utils.js",
+      "web-null_l10n": "../web/genericl10n.js",
     },
   };
   const licenseHeaderLibre = fs

diff --git a/test/unit/pdf_viewer.component_spec.js b/test/unit/pdf_viewer.component_spec.js
@@ -30,8 +30,6 @@ import { AnnotationLayerBuilder } from "../../web/annotation_layer_builder.js";
 import { DownloadManager } from "../../web/download_manager.js";
 import { EventBus } from "../../web/event_utils.js";
 import { GenericL10n } from "../../web/genericl10n.js";
-import { L10n } from "../../web/l10n.js";
-import { NullL10n } from "../../web/l10n_utils.js";
 import { PDFHistory } from "../../web/pdf_history.js";
 import { PDFPageView } from "../../web/pdf_page_view.js";
 import { PDFScriptingManager } from "../../web/pdf_scripting_manager.component.js";
@@ -54,7 +52,6 @@ describe("pdfviewer_api", function () {
       FindState,
       GenericL10n,
       LinkTarget,
-      NullL10n,
       parseQueryString,
       PDFFindController,
       PDFHistory,
@@ -73,14 +70,4 @@ describe("pdfviewer_api", function () {
       XfaLayerBuilder,
     });
   });
-
-  it("checks that `NullL10n` implements all methods", function () {
-    const methods = Object.getOwnPropertyNames(NullL10n).sort();
-
-    const baseMethods = Object.getOwnPropertyNames(L10n.prototype)
-      .filter(m => m !== "constructor" && !m.startsWith("_"))
-      .sort();
-
-    expect(methods).toEqual(baseMethods);
-  });
 });
diff --git a/test/unit/unit_test.html b/test/unit/unit_test.html
@@ -30,7 +30,7 @@
         "web-com": "../../web/genericcom.js",
         "web-download_manager": "../../web/download_manager.js",
         "web-external_services": "../../web/genericcom.js",
-        "web-l10n_utils": "../../web/l10n_utils.js",
+        "web-null_l10n": "../../web/genericl10n.js",
         "web-pdf_attachment_viewer": "../../web/pdf_attachment_viewer.js",
         "web-pdf_cursor_tools": "../../web/pdf_cursor_tools.js",
         "web-pdf_document_properties": "../../web/pdf_document_properties.js",

diff --git a/tsconfig.json b/tsconfig.json
@@ -16,7 +16,7 @@
       "display-node_utils": ["./src/display/node_utils"],
       "fluent-bundle": ["./node_modules/@fluent/bundle/esm/index.js"],
       "fluent-dom": ["./node_modules/@fluent/dom/esm/index.js"],
-      "web-l10n_utils": ["./web/l10n_utils"]
+      "web-null_l10n": ["../web/genericl10n.js"]
     }
   },
   "files": ["src/pdf.js", "web/pdf_viewer.component.js"]

diff --git a/web/annotation_editor_layer_builder.js b/web/annotation_editor_layer_builder.js
@@ -25,7 +25,7 @@
 /** @typedef {import("../src/display/annotation_layer.js").AnnotationLayer} AnnotationLayer */
 
 import { AnnotationEditorLayer } from "pdfjs-lib";
-import { NullL10n } from "web-l10n_utils";
+import { NullL10n } from "web-null_l10n";
 
 /**
  * @typedef {Object} AnnotationEditorLayerBuilderOptions
@@ -55,7 +55,10 @@ class AnnotationEditorLayerBuilder {
     this.pageDiv = options.pageDiv;
     this.pdfPage = options.pdfPage;
     this.accessibilityManager = options.accessibilityManager;
-    this.l10n = options.l10n || NullL10n;
+    this.l10n = options.l10n;
+    if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
+      this.l10n ||= new NullL10n();
+    }
     this.annotationEditorLayer = null;
     this.div = null;
     this._cancelled = false;

diff --git a/web/genericl10n.js b/web/genericl10n.js
@@ -20,6 +20,27 @@ import { DOMLocalization } from "fluent-dom";
 import { fetchData } from "pdfjs-lib";
 import { L10n } from "./l10n.js";
 
+async function createBundleFallback(lang) {
+  if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
+    throw new Error("Not implemented in TESTING builds: createBundleFallback");
+  }
+  const text =
+    typeof PDFJSDev === "undefined"
+      ? await fetchData(
+          new URL(`./locale/${lang}/viewer.ftl`, window.location.href),
+          /* type = */ "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);
+  }
+  return bundle;
+}
+
 /**
  * @implements {IL10n}
  */
@@ -63,6 +84,9 @@ class GenericL10n extends L10n {
       if (bundle) {
         yield bundle;
       }
+      if (lang === "en-us") {
+        yield createBundleFallback(lang);
+      }
     }
   }
 
@@ -84,11 +108,33 @@ class GenericL10n extends L10n {
   }
 
   static async #getPaths() {
-    const { href } = document.querySelector(`link[type="application/l10n"]`);
-    const paths = await fetchData(href, /* type = */ "json");
+    try {
+      const { href } = document.querySelector(`link[type="application/l10n"]`);
+      const paths = await fetchData(href, /* type = */ "json");
+
+      return { baseURL: href.replace(/[^/]*$/, "") || "./", paths };
+    } catch {}
+    return { baseURL: "./", paths: Object.create(null) };
+  }
+}
+
+/**
+ * @implements {IL10n}
+ */
+class NullL10n extends L10n {
+  constructor() {
+    super({ lang: "en-us" });
+    this._setL10n(
+      new DOMLocalization(
+        [],
+        NullL10n.#generateBundles.bind(NullL10n, this.getLanguage())
+      )
+    );
+  }
 
-    return { baseURL: href.replace(/[^/]*$/, "") || "./", paths };
+  static async *#generateBundles(lang) {
+    yield createBundleFallback(lang);
   }
 }
 
-export { GenericL10n };
+export { GenericL10n, NullL10n };
diff --git a/web/l10n.js b/web/l10n.js
@@ -117,4 +117,6 @@ class L10n {
   }
 }
 
-export { L10n };
+const NullL10n = null;
+
+export { L10n, NullL10n };
diff --git a/web/l10n_utils.js b/web/l10n_utils.js
diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js
@@ -43,7 +43,7 @@ import { AnnotationEditorLayerBuilder } from "./annotation_editor_layer_builder.
 import { AnnotationLayerBuilder } from "./annotation_layer_builder.js";
 import { compatibilityParams } from "./app_options.js";
 import { DrawLayerBuilder } from "./draw_layer_builder.js";
-import { NullL10n } from "web-l10n_utils";
+import { NullL10n } from "web-null_l10n";
 import { SimpleLinkService } from "./pdf_link_service.js";
 import { StructTreeLayerBuilder } from "./struct_tree_layer_builder.js";
 import { TextAccessibilityManager } from "./text_accessibility.js";
@@ -157,7 +157,10 @@ class PDFPageView {
 
     this.eventBus = options.eventBus;
     this.renderingQueue = options.renderingQueue;
-    this.l10n = options.l10n || NullL10n;
+    this.l10n = options.l10n;
+    if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
+      this.l10n ||= new NullL10n();
+    }
 
     this.renderTask = null;
     this.resume = null;
@@ -214,7 +217,7 @@ class PDFPageView {
       }
 
       // Ensure that Fluent is connected in e.g. the COMPONENTS build.
-      if (this.l10n === NullL10n) {
+      if (this.l10n instanceof NullL10n) {
         this.l10n.translate(this.div);
       }
     }

diff --git a/web/pdf_viewer.component.js b/web/pdf_viewer.component.js
@@ -30,7 +30,6 @@ import { AnnotationLayerBuilder } from "./annotation_layer_builder.js";
 import { DownloadManager } from "./download_manager.js";
 import { EventBus } from "./event_utils.js";
 import { GenericL10n } from "./genericl10n.js";
-import { NullL10n } from "./l10n_utils.js";
 import { PDFHistory } from "./pdf_history.js";
 import { PDFPageView } from "./pdf_page_view.js";
 import { PDFScriptingManager } from "./pdf_scripting_manager.component.js";
@@ -54,7 +53,6 @@ export {
   FindState,
   GenericL10n,
   LinkTarget,
-  NullL10n,
   parseQueryString,
   PDFFindController,
   PDFHistory,

diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js
@@ -63,7 +63,7 @@ import {
   VERTICAL_PADDING,
   watchScroll,
 } from "./ui_utils.js";
-import { NullL10n } from "web-l10n_utils";
+import { NullL10n } from "web-null_l10n";
 import { PDFPageView } from "./pdf_page_view.js";
 import { PDFRenderingQueue } from "./pdf_rendering_queue.js";
 import { SimpleLinkService } from "./pdf_link_service.js";
@@ -286,7 +286,10 @@ class PDFViewer {
       this.removePageBorders = options.removePageBorders || false;
     }
     this.maxCanvasPixels = options.maxCanvasPixels;
-    this.l10n = options.l10n || NullL10n;
+    this.l10n = options.l10n;
+    if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
+      this.l10n ||= new NullL10n();
+    }
     this.#enablePermissions = options.enablePermissions || false;
     this.pageColors = options.pageColors || null;
 
@@ -327,7 +330,7 @@ class PDFViewer {
 
     if (
       (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
-      this.l10n === NullL10n
+      this.l10n instanceof NullL10n
     ) {
       // Ensure that Fluent is connected in e.g. the COMPONENTS build.
       this.l10n.translate(this.container);

diff --git a/web/stubs.js b/web/stubs.js