From 39251c578920562aa62c59d49555930af10e1a5f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 8 May 2022 10:51:59 +0200 Subject: [PATCH 1/2] Re-order the names of the new `pageColors` options/preferences (PR 14874 follow-up) Given that the new API-option is an Object named `pageColors`, with `background`/`foreground` keys, it occurred to me that it'd be slightly more consistent if the options/preferences names fully reflected that. --- extensions/chromium/preferences_schema.json | 4 ++-- web/app.js | 4 ++-- web/app_options.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/extensions/chromium/preferences_schema.json b/extensions/chromium/preferences_schema.json index 6f7f2004f..86d9388f3 100644 --- a/extensions/chromium/preferences_schema.json +++ b/extensions/chromium/preferences_schema.json @@ -209,12 +209,12 @@ ], "default": -1 }, - "pageBackgroundColor": { + "pageColorsBackground": { "description": "The color is a string as defined in CSS. Its goal is to help improve readability in high contrast mode", "type": "string", "default": "Canvas" }, - "pageForegroundColor": { + "pageColorsForeground": { "description": "The color is a string as defined in CSS. Its goal is to help improve readability in high contrast mode", "type": "string", "default": "CanvasText" diff --git a/web/app.js b/web/app.js index 532ddb1a0..7c22e9c20 100644 --- a/web/app.js +++ b/web/app.js @@ -526,8 +526,8 @@ const PDFViewerApplication = { maxCanvasPixels: AppOptions.get("maxCanvasPixels"), enablePermissions: AppOptions.get("enablePermissions"), pageColors: { - background: AppOptions.get("pageBackgroundColor"), - foreground: AppOptions.get("pageForegroundColor"), + background: AppOptions.get("pageColorsBackground"), + foreground: AppOptions.get("pageColorsForeground"), }, }); pdfRenderingQueue.setViewer(this.pdfViewer); diff --git a/web/app_options.js b/web/app_options.js index e73e0c429..205f826cd 100644 --- a/web/app_options.js +++ b/web/app_options.js @@ -129,12 +129,12 @@ const defaultOptions = { compatibility: compatibilityParams.maxCanvasPixels, kind: OptionKind.VIEWER, }, - pageBackgroundColor: { + pageColorsBackground: { /** @type {string} */ value: "Canvas", kind: OptionKind.VIEWER + OptionKind.PREFERENCE, }, - pageForegroundColor: { + pageColorsForeground: { /** @type {string} */ value: "CanvasText", kind: OptionKind.VIEWER + OptionKind.PREFERENCE, From 472a1f9c91650448e2f3a5bdb235972e505774e6 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 8 May 2022 11:06:18 +0200 Subject: [PATCH 2/2] Ignore `pageColors` when the background/foreground is identical (PR 14874 follow-up) If the computed background/foreground colors are identical, the `canvas` would be rendered mostly blank with only images visible. Hence it seems reasonable to also ignore the `pageColors`-option in this case. Also, the patch tries to *briefly* outline the various cases in which we ignore the `pageColors`-option in a comment. --- src/display/canvas.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/display/canvas.js b/src/display/canvas.js index b34aba22e..469cc588b 100644 --- a/src/display/canvas.js +++ b/src/display/canvas.js @@ -1302,7 +1302,19 @@ class CanvasGraphics { typeof defaultBg === "string" && /^#[0-9A-Fa-f]{6}$/.test(defaultBg); } - if ((fg === "#000000" && bg === "#ffffff") || !isValidDefaultBg) { + if ( + (fg === "#000000" && bg === "#ffffff") || + fg === bg || + !isValidDefaultBg + ) { + // Ignore the `pageColors`-option when: + // - The computed background/foreground colors have their default + // values, i.e. white/black. + // - The computed background/foreground colors are identical, + // since that'd render the `canvas` mostly blank. + // - The `background`-option has a value that's incompatible with + // the `pageColors`-values. + // this.foregroundColor = this.backgroundColor = null; } else { // https://developer.mozilla.org/en-US/docs/Web/Accessibility/Understanding_Colors_and_Luminance