Skip to content

Commit

Permalink
[api-minor] Re-factor NullL10n and remove the hard-coded l10n strin…
Browse files Browse the repository at this point in the history
…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.)
  • Loading branch information
Snuffleupagus committed Oct 20, 2023
1 parent a4cd2ef commit f07675a
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 125 deletions.
20 changes: 17 additions & 3 deletions gulpfile.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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/";

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -1572,6 +1585,7 @@ function buildLib(defines, dir) {
DEFAULT_PREFERENCES: getDefaultPreferences(
defines.SKIP_BABEL ? "lib/" : "lib-legacy/"
),
DEFAULT_FTL: getDefaultFtl(),
};

const inputStream = merge([
Expand Down
15 changes: 0 additions & 15 deletions src/display/annotation_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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", {
Expand Down Expand Up @@ -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);
}
}

/**
Expand Down
2 changes: 0 additions & 2 deletions src/display/stubs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,7 +26,6 @@ export {
NodeCMapReaderFactory,
NodeFilterFactory,
NodeStandardFontDataFactory,
NullL10n,
PDFFetchStream,
PDFNetworkStream,
PDFNodeStream,
Expand Down
1 change: 0 additions & 1 deletion test/unit/unit_test.html
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
4 changes: 2 additions & 2 deletions web/interfaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
139 changes: 40 additions & 99 deletions web/l10n_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,107 +13,46 @@
* 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, lang))
);
}

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 + "}";
});
}

/**
Expand All @@ -122,18 +61,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 };
5 changes: 5 additions & 0 deletions web/pdf_page_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions web/pdf_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 0 additions & 1 deletion web/viewer-geckoview.html
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion web/viewer.html
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit f07675a

Please sign in to comment.