Merge pull request #17476 from calixteman/bug1869001

Avoid to have the text layer mismatching the rendered text with mismatching locales (bug 1869001)
2024-01-04 22:03:35 +01:00 · 2024-01-04 22:03:35 +01:00 · 1019b9f821
commit 1019b9f821
parent 7873ad98bb f84f48b5d0
7 changed files with 42 additions and 46 deletions
--- 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() {
--- 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();
+let _canvasContext = null;

-function getCtx(size, isOffscreenCanvasSupported) {
-  let ctx;
-  if (isOffscreenCanvasSupported && FeatureTest.isOffscreenCanvasSupported) {
-    ctx = new OffscreenCanvas(size, size).getContext("2d", { alpha: false });
-  } else {
+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 getAscent(fontFamily, isOffscreenCanvasSupported) {
+function cleanupTextLayer() {
+  _canvasContext?.canvas.remove();
+  _canvasContext = null;
+}
+
+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,
+};
--- 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,
--- 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,
      });
--- 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;
--- 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,
--- 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;