Merge pull request #17476 from calixteman/bug1869001

Avoid to have the text layer mismatching the rendered text with mismatching locales (bug 1869001)
mozilla · Jan 4, 2024 · 1019b9f · 1019b9f
2 parents 7873ad9 + f84f48b
commit 1019b9f
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;