Avoid to have the text layer mismatching the rendered text with misma…

…tching locales (bug 1869001) The system locale (used in OffscreenCanvas) can be different from the one guessed by Fluent, consequently, in order to avoid any mismatch, we just use an attached canvas element. The original issue can easily be reproduced locally in adding a lang="ja" in viewer.html (or with an other language for Japanese users).
mozilla · Jan 4, 2024 · f84f48b · f84f48b
1 parent 7873ad9
commit f84f48b
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 47 deletions.
diff --git a/src/display/api.js b/src/display/api.js
@@ -63,6 +63,7 @@ import {
   NodeStandardFontDataFactory,
 } from "display-node_utils";
 import { CanvasGraphics } from "./canvas.js";
+import { cleanupTextLayer } from "./text_layer.js";
 import { GlobalWorkerOptions } from "./worker_options.js";
 import { MessageHandler } from "../shared/message_handler.js";
 import { Metadata } from "./metadata.js";
@@ -2481,6 +2482,7 @@ class WorkerTransport {
       this.fontLoader.clear();
       this.#methodPromises.clear();
       this.filterFactory.destroy();
+      cleanupTextLayer();
 
       this._networkStream?.cancelAllRequests(
         new AbortException("Worker was terminated.")
@@ -3065,6 +3067,7 @@ class WorkerTransport {
     }
     this.#methodPromises.clear();
     this.filterFactory.destroy(/* keepHCM = */ true);
+    cleanupTextLayer();
   }
 
   get loadingParams() {

diff --git a/src/display/text_layer.js b/src/display/text_layer.js
@@ -16,12 +16,7 @@
 /** @typedef {import("./display_utils").PageViewport} PageViewport */
 /** @typedef {import("./api").TextContent} TextContent */
 
-import {
-  AbortException,
-  FeatureTest,
-  PromiseCapability,
-  Util,
-} from "../shared/util.js";
+import { AbortException, PromiseCapability, Util } from "../shared/util.js";
 import { setLayerDimensions } from "./display_utils.js";
 
 /**
@@ -43,8 +38,6 @@ import { setLayerDimensions } from "./display_utils.js";
  * @property {Array<string>} [textContentItemsStr] - Strings that correspond to
  *   the `str` property of the text items of the textContent input.
  *   This is output and shall initially be set to an empty array.
- * @property {boolean} [isOffscreenCanvasSupported] true if we can use
- *   OffscreenCanvas to measure string widths.
  */
 
 /**
@@ -60,8 +53,6 @@ import { setLayerDimensions } from "./display_utils.js";
  *   This is output and shall initially be set to an empty array.
  * @property {WeakMap<HTMLElement,Object>} [textDivProperties] - Some properties
  *   weakly mapped to the HTML elements used to render the text.
- * @property {boolean} [isOffscreenCanvasSupported] true if we can use
- *   OffscreenCanvas to measure string widths.
  * @property {boolean} [mustRotate] true if the text layer must be rotated.
  * @property {boolean} [mustRescale] true if the text layer contents must be
  *   rescaled.
@@ -71,28 +62,43 @@ const MAX_TEXT_DIVS_TO_RENDER = 100000;
 const DEFAULT_FONT_SIZE = 30;
 const DEFAULT_FONT_ASCENT = 0.8;
 const ascentCache = new Map();
-
-function getCtx(size, isOffscreenCanvasSupported) {
-  let ctx;
-  if (isOffscreenCanvasSupported && FeatureTest.isOffscreenCanvasSupported) {
-    ctx = new OffscreenCanvas(size, size).getContext("2d", { alpha: false });
-  } else {
+let _canvasContext = null;
+
+function getCtx() {
+  if (!_canvasContext) {
+    // We don't use an OffscreenCanvas here because we use serif/sans serif
+    // fonts with it and they depends on the locale.
+    // In Firefox, the <html> element get a lang attribute that depends on what
+    // Fluent returns for the locale and the OffscreenCanvas uses the OS locale.
+    // Those two locales can be different and consequently the used fonts will
+    // be different (see bug 1869001).
+    // Ideally, we should use in the text layer the fonts we've in the pdf (or
+    // their replacements when they aren't embedded) and then we can use an
+    // OffscreenCanvas.
     const canvas = document.createElement("canvas");
-    canvas.width = canvas.height = size;
-    ctx = canvas.getContext("2d", { alpha: false });
+    canvas.className = "hiddenCanvasElement";
+    document.body.append(canvas);
+    _canvasContext = canvas.getContext("2d", { alpha: false });
   }
 
-  return ctx;
+  return _canvasContext;
+}
+
+function cleanupTextLayer() {
+  _canvasContext?.canvas.remove();
+  _canvasContext = null;
 }
 
-function getAscent(fontFamily, isOffscreenCanvasSupported) {
+function getAscent(fontFamily) {
   const cachedAscent = ascentCache.get(fontFamily);
   if (cachedAscent) {
     return cachedAscent;
   }
 
-  const ctx = getCtx(DEFAULT_FONT_SIZE, isOffscreenCanvasSupported);
+  const ctx = getCtx();
 
+  const savedFont = ctx.font;
+  ctx.canvas.width = ctx.canvas.height = DEFAULT_FONT_SIZE;
   ctx.font = `${DEFAULT_FONT_SIZE}px ${fontFamily}`;
   const metrics = ctx.measureText("");
 
@@ -104,6 +110,7 @@ function getAscent(fontFamily, isOffscreenCanvasSupported) {
     ascentCache.set(fontFamily, ratio);
 
     ctx.canvas.width = ctx.canvas.height = 0;
+    ctx.font = savedFont;
     return ratio;
   }
 
@@ -143,6 +150,7 @@ function getAscent(fontFamily, isOffscreenCanvasSupported) {
   }
 
   ctx.canvas.width = ctx.canvas.height = 0;
+  ctx.font = savedFont;
 
   if (ascent) {
     const ratio = ascent / (ascent + descent);
@@ -176,8 +184,7 @@ function appendText(task, geom, styles) {
   const fontFamily =
     (task._fontInspectorEnabled && style.fontSubstitution) || style.fontFamily;
   const fontHeight = Math.hypot(tx[2], tx[3]);
-  const fontAscent =
-    fontHeight * getAscent(fontFamily, task._isOffscreenCanvasSupported);
+  const fontAscent = fontHeight * getAscent(fontFamily);
 
   let left, top;
   if (angle === 0) {
@@ -308,14 +315,12 @@ class TextLayerRenderTask {
     textDivs,
     textDivProperties,
     textContentItemsStr,
-    isOffscreenCanvasSupported,
   }) {
     this._textContentSource = textContentSource;
     this._isReadableStream = textContentSource instanceof ReadableStream;
     this._container = this._rootContainer = container;
     this._textDivs = textDivs || [];
     this._textContentItemsStr = textContentItemsStr || [];
-    this._isOffscreenCanvasSupported = isOffscreenCanvasSupported;
     this._fontInspectorEnabled = !!globalThis.FontInspector?.enabled;
 
     this._reader = null;
@@ -328,7 +333,7 @@ class TextLayerRenderTask {
       div: null,
       scale: viewport.scale * (globalThis.devicePixelRatio || 1),
       properties: null,
-      ctx: getCtx(0, isOffscreenCanvasSupported),
+      ctx: getCtx(),
     };
     const { pageWidth, pageHeight, pageX, pageY } = viewport.rawDims;
     this._transform = [1, 0, 0, -1, -pageX, pageY + pageHeight];
@@ -474,7 +479,6 @@ function updateTextLayer({
   viewport,
   textDivs,
   textDivProperties,
-  isOffscreenCanvasSupported,
   mustRotate = true,
   mustRescale = true,
 }) {
@@ -483,7 +487,7 @@ function updateTextLayer({
   }
 
   if (mustRescale) {
-    const ctx = getCtx(0, isOffscreenCanvasSupported);
+    const ctx = getCtx();
     const scale = viewport.scale * (globalThis.devicePixelRatio || 1);
     const params = {
       prevFontSize: null,
@@ -501,4 +505,9 @@ function updateTextLayer({
   }
 }
 
-export { renderTextLayer, TextLayerRenderTask, updateTextLayer };
+export {
+  cleanupTextLayer,
+  renderTextLayer,
+  TextLayerRenderTask,
+  updateTextLayer,
+};
diff --git a/web/app.js b/web/app.js
@@ -441,7 +441,6 @@ const PDFViewerApplication = {
       annotationEditorHighlightColors: AppOptions.get("highlightEditorColors"),
       imageResourcesPath: AppOptions.get("imageResourcesPath"),
       enablePrintAutoRotate: AppOptions.get("enablePrintAutoRotate"),
-      isOffscreenCanvasSupported,
       maxCanvasPixels: AppOptions.get("maxCanvasPixels"),
       enablePermissions: AppOptions.get("enablePermissions"),
       pageColors,

diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js
@@ -72,8 +72,6 @@ import { XfaLayerBuilder } from "./xfa_layer_builder.js";
  *   The default value is `AnnotationMode.ENABLE_FORMS`.
  * @property {string} [imageResourcesPath] - Path for image resources, mainly
  *   for annotation icons. Include trailing slash.
- * @property {boolean} [isOffscreenCanvasSupported] - Allows to use an
- *   OffscreenCanvas if needed.
  * @property {number} [maxCanvasPixels] - The maximum supported canvas size in
  *   total pixels, i.e. width * height. Use `-1` for no limit, or `0` for
  *   CSS-only zooming. The default value is 4096 * 4096 (16 mega-pixels).
@@ -154,8 +152,6 @@ class PDFPageView {
     this.#annotationMode =
       options.annotationMode ?? AnnotationMode.ENABLE_FORMS;
     this.imageResourcesPath = options.imageResourcesPath || "";
-    this.isOffscreenCanvasSupported =
-      options.isOffscreenCanvasSupported ?? true;
     this.maxCanvasPixels = options.maxCanvasPixels ?? MAX_CANVAS_PIXELS;
     this.pageColors = options.pageColors || null;
 
@@ -879,7 +875,6 @@ class PDFPageView {
       this.textLayer = new TextLayerBuilder({
         highlighter: this._textHighlighter,
         accessibilityManager: this._accessibilityManager,
-        isOffscreenCanvasSupported: this.isOffscreenCanvasSupported,
         enablePermissions:
           this.#textLayerMode === TextLayerMode.ENABLE_PERMISSIONS,
       });

diff --git a/web/pdf_viewer.css b/web/pdf_viewer.css
@@ -46,7 +46,8 @@
   transform: rotate(270deg) translateX(-100%);
 }
 
-#hiddenCopyElement {
+#hiddenCopyElement,
+.hiddenCanvasElement {
   position: absolute;
   top: 0;
   left: 0;

diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js
@@ -115,8 +115,6 @@ function isValidAnnotationEditorMode(mode) {
  *   mainly for annotation icons. Include trailing slash.
  * @property {boolean} [enablePrintAutoRotate] - Enables automatic rotation of
  *   landscape pages upon printing. The default is `false`.
- * @property {boolean} [isOffscreenCanvasSupported] - Allows to use an
- *   OffscreenCanvas if needed.
  * @property {number} [maxCanvasPixels] - The maximum supported canvas size in
  *   total pixels, i.e. width * height. Use `-1` for no limit, or `0` for
  *   CSS-only zooming. The default value is 4096 * 4096 (16 mega-pixels).
@@ -287,8 +285,6 @@ class PDFViewer {
     if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
       this.removePageBorders = options.removePageBorders || false;
     }
-    this.isOffscreenCanvasSupported =
-      options.isOffscreenCanvasSupported ?? true;
     this.maxCanvasPixels = options.maxCanvasPixels;
     this.l10n = options.l10n || NullL10n;
     this.#enablePermissions = options.enablePermissions || false;
@@ -919,7 +915,6 @@ class PDFViewer {
             textLayerMode,
             annotationMode,
             imageResourcesPath: this.imageResourcesPath,
-            isOffscreenCanvasSupported: this.isOffscreenCanvasSupported,
             maxCanvasPixels: this.maxCanvasPixels,
             pageColors: this.pageColors,
             l10n: this.l10n,

diff --git a/web/text_layer_builder.js b/web/text_layer_builder.js
@@ -28,8 +28,6 @@ import { removeNullCharacters } from "./ui_utils.js";
  * @property {TextHighlighter} highlighter - Optional object that will handle
  *   highlighting text from the find controller.
  * @property {TextAccessibilityManager} [accessibilityManager]
- * @property {boolean} [isOffscreenCanvasSupported] - Allows to use an
- *   OffscreenCanvas if needed.
  */
 
 /**
@@ -49,7 +47,6 @@ class TextLayerBuilder {
   constructor({
     highlighter = null,
     accessibilityManager = null,
-    isOffscreenCanvasSupported = true,
     enablePermissions = false,
   }) {
     this.textContentItemsStr = [];
@@ -59,7 +56,6 @@ class TextLayerBuilder {
     this.textLayerRenderTask = null;
     this.highlighter = highlighter;
     this.accessibilityManager = accessibilityManager;
-    this.isOffscreenCanvasSupported = isOffscreenCanvasSupported;
     this.#enablePermissions = enablePermissions === true;
 
     /**
@@ -107,7 +103,6 @@ class TextLayerBuilder {
           viewport,
           textDivs: this.textDivs,
           textDivProperties: this.textDivProperties,
-          isOffscreenCanvasSupported: this.isOffscreenCanvasSupported,
           mustRescale,
           mustRotate,
         });
@@ -129,7 +124,6 @@ class TextLayerBuilder {
       textDivs: this.textDivs,
       textDivProperties: this.textDivProperties,
       textContentItemsStr: this.textContentItemsStr,
-      isOffscreenCanvasSupported: this.isOffscreenCanvasSupported,
     });
 
     await this.textLayerRenderTask.promise;