From f84f48b5d0f8d82a0f5ac344c6e9cdb4f13b0097 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Thu, 4 Jan 2024 15:53:36 +0100 Subject: [PATCH] Avoid to have the text layer mismatching the rendered text with mismatching 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). --- src/display/api.js | 3 ++ src/display/text_layer.js | 65 ++++++++++++++++++++++----------------- web/app.js | 1 - web/pdf_page_view.js | 5 --- web/pdf_viewer.css | 3 +- web/pdf_viewer.js | 5 --- web/text_layer_builder.js | 6 ---- 7 files changed, 42 insertions(+), 46 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 037c7d0da..e0ba09d45 100644 --- 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 index d974ba8b6..22be2748c 100644 --- 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} [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} [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 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, +}; diff --git a/web/app.js b/web/app.js index f8077c4ce..36ac82814 100644 --- 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 index d31f50b39..08c8e23e1 100644 --- 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 index 0aa7cb5cb..3c8a29271 100644 --- 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 index 75dc069aa..16484671b 100644 --- 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 index d22ce6cc4..dc676bf6d 100644 --- 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;