From b3ea789286f346dff43ca50ae0ebc3d4262a4785 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 22 Jul 2024 09:41:37 +0200 Subject: [PATCH] Add better support for AppOptions with multiple types (PR 18465 follow-up) For options with varying types, see `useSystemFonts`, we're not sufficiently validating the type when setting a new value. This means that for an option that uses `OptionKind.UNDEF_ALLOWED` we'd allow *any* value, which is obviously not the intention. Hence we instead introduce a new and *optional* `type`-field that allows specifying exactly which types are valid when multiple ones are supported. *Note:* This obviously didn't occur to me until after PR 18465 landed, sorry about that! --- web/app_options.js | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/web/app_options.js b/web/app_options.js index 66b36361e97cc..456f5ce26c8b9 100644 --- a/web/app_options.js +++ b/web/app_options.js @@ -55,10 +55,18 @@ const OptionKind = { API: 0x04, WORKER: 0x08, EVENT_DISPATCH: 0x10, - UNDEF_ALLOWED: 0x20, PREFERENCE: 0x80, }; +// Should only be used with options that allow multiple types. +const Type = { + BOOLEAN: 0x01, + NUMBER: 0x02, + OBJECT: 0x04, + STRING: 0x08, + UNDEFINED: 0x10, +}; + /** * NOTE: These options are used to generate the `default_preferences.json` file, * see `OptionKind.PREFERENCE`, hence the values below must use only @@ -405,7 +413,8 @@ const defaultOptions = { ) ? false : undefined, - kind: OptionKind.API + OptionKind.UNDEF_ALLOWED, + kind: OptionKind.API, + type: Type.BOOLEAN + Type.UNDEFINED, }, verbosity: { /** @type {number} */ @@ -484,7 +493,7 @@ if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) { if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING || LIB")) { // Ensure that the `defaultOptions` are correctly specified. for (const name in defaultOptions) { - const { value, kind } = defaultOptions[name]; + const { value, kind, type } = defaultOptions[name]; if (kind & OptionKind.PREFERENCE) { if (kind === OptionKind.PREFERENCE) { @@ -493,9 +502,9 @@ if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING || LIB")) { if (kind & OptionKind.BROWSER) { throw new Error(`Cannot mix "PREFERENCE" and "BROWSER" kind: ${name}`); } - if (kind & OptionKind.UNDEF_ALLOWED) { + if (type !== undefined) { throw new Error( - `Cannot allow \`undefined\` value for "PREFERENCE" kind: ${name}` + `Cannot have \`type\`-field for "PREFERENCE" kind: ${name}` ); } if (typeof compatParams === "object" && compatParams.has(name)) { @@ -512,9 +521,9 @@ if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING || LIB")) { throw new Error(`Invalid value for "PREFERENCE" kind: ${name}`); } } else if (kind & OptionKind.BROWSER) { - if (kind & OptionKind.UNDEF_ALLOWED) { + if (type !== undefined) { throw new Error( - `Cannot allow \`undefined\` value for "BROWSER" kind: ${name}` + `Cannot have \`type\`-field for "BROWSER" kind: ${name}` ); } if (typeof compatParams === "object" && compatParams.has(name)) { @@ -565,8 +574,7 @@ class AppOptions { !defaultOpt || !( typeof value === typeof defaultOpt.value || - (defaultOpt.kind & OptionKind.UNDEF_ALLOWED && - (value === undefined || defaultOpt.value === undefined)) + Type[(typeof value).toUpperCase()] & defaultOpt.type ) ) { return; @@ -585,8 +593,7 @@ class AppOptions { !defaultOpt || !( typeof userOpt === typeof defaultOpt.value || - (defaultOpt.kind & OptionKind.UNDEF_ALLOWED && - (userOpt === undefined || defaultOpt.value === undefined)) + Type[(typeof userOpt).toUpperCase()] & defaultOpt.type ) ) { continue;