Skip to content

Commit

Permalink
Re-factor BasePreferences to essentially be a wrapper around `AppOp…
Browse files Browse the repository at this point in the history 
…tions`

In the MOZCENTRAL build the `BasePreferences` class is almost unused, since it's only being used to initialize and update the `AppOptions` which means that theoretically we could move the relevant code there.
However for the other build targets (e.g. GENERIC and CHROME) we still need to keep the `BasePreferences` class, although we can re-factor things to move the necessary validation inside of `AppOptions` and thus simplify the code and reduce duplication.
  • Loading branch information
@Snuffleupagus
Snuffleupagus committed Jul 9, 2024
1 parent 1bdd692 commit e04ac83
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 123 deletions.
24 changes: 0 additions & 24 deletions gulpfile.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,6 @@ function createWebpackConfig(
BUNDLE_VERSION: versionInfo.version,
BUNDLE_BUILD: versionInfo.commit,
TESTING: defines.TESTING ?? process.env.TESTING === "true",
BROWSER_PREFERENCES: defaultPreferencesDir
? getBrowserPreferences(defaultPreferencesDir)
: {},
DEFAULT_PREFERENCES: defaultPreferencesDir
? getDefaultPreferences(defaultPreferencesDir)
: {},
Expand Down Expand Up @@ -856,13 +853,6 @@ async function parseDefaultPreferences(dir) {
"./" + DEFAULT_PREFERENCES_DIR + dir + "app_options.mjs"
);

const browserPrefs = AppOptions.getAll(
OptionKind.BROWSER,
/* defaultOnly = */ true
);
if (Object.keys(browserPrefs).length === 0) {
throw new Error("No browser preferences found.");
}
const prefs = AppOptions.getAll(
OptionKind.PREFERENCE,
/* defaultOnly = */ true
Expand All @@ -871,23 +861,12 @@ async function parseDefaultPreferences(dir) {
throw new Error("No default preferences found.");
}

fs.writeFileSync(
DEFAULT_PREFERENCES_DIR + dir + "browser_preferences.json",
JSON.stringify(browserPrefs)
);
fs.writeFileSync(
DEFAULT_PREFERENCES_DIR + dir + "default_preferences.json",
JSON.stringify(prefs)
);
}

function getBrowserPreferences(dir) {
const str = fs
.readFileSync(DEFAULT_PREFERENCES_DIR + dir + "browser_preferences.json")
.toString();
return JSON.parse(str);
}

function getDefaultPreferences(dir) {
const str = fs
.readFileSync(DEFAULT_PREFERENCES_DIR + dir + "default_preferences.json")
Expand Down Expand Up @@ -1581,9 +1560,6 @@ function buildLib(defines, dir) {
BUNDLE_VERSION: versionInfo.version,
BUNDLE_BUILD: versionInfo.commit,
TESTING: defines.TESTING ?? process.env.TESTING === "true",
BROWSER_PREFERENCES: getBrowserPreferences(
defines.SKIP_BABEL ? "lib/" : "lib-legacy/"
),
DEFAULT_PREFERENCES: getDefaultPreferences(
defines.SKIP_BABEL ? "lib/" : "lib-legacy/"
),
Expand Down
54 changes: 36 additions & 18 deletions web/app_options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,27 +488,23 @@ class AppOptions {
userOptions[name] = value;
}

static setAll(options, init = false) {
if ((typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) && init) {
if (this.get("disablePreferences")) {
// Give custom implementations of the default viewer a simpler way to
// opt-out of having the `Preferences` override existing `AppOptions`.
return;
}
for (const name in userOptions) {
// Ignore any compatibility-values in the user-options.
if (compatibilityParams[name] !== undefined) {
static setAll(options, prefs = false) {
for (const name in options) {
if (prefs) {
const defaultOption = defaultOptions[name];

if (!defaultOption) {
continue;
}
console.warn(
"setAll: The Preferences may override manually set AppOptions; " +
'please use the "disablePreferences"-option in order to prevent that.'
);
break;
}
}
const { kind, value } = defaultOption;

for (const name in options) {
if (!(kind & OptionKind.BROWSER || kind & OptionKind.PREFERENCE)) {
continue;
}
if (typeof options[name] !== typeof value) {
continue;
}
}
userOptions[name] = options[name];
}
}
Expand All @@ -526,4 +522,26 @@ class AppOptions {
}
}

if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
AppOptions._checkDisablePreferences = () => {
if (AppOptions.get("disablePreferences")) {
// Give custom implementations of the default viewer a simpler way to
// opt-out of having the `Preferences` override existing `AppOptions`.
return true;
}
for (const name in userOptions) {
// Ignore any compatibility-values in the user-options.
if (compatibilityParams[name] !== undefined) {
continue;
}
console.warn(
"setAll: The Preferences may override manually set AppOptions; " +
'please use the "disablePreferences"-option in order to prevent that.'
);
break;
}
return false;
};
}

export { AppOptions, OptionKind };
99 changes: 18 additions & 81 deletions web/preferences.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,12 @@ import { AppOptions, OptionKind } from "./app_options.js";
* or every time the viewer is loaded.
*/
class BasePreferences {
#browserDefaults = Object.freeze(
typeof PDFJSDev === "undefined"
? AppOptions.getAll(OptionKind.BROWSER, /* defaultOnly = */ true)
: PDFJSDev.eval("BROWSER_PREFERENCES")
);

#defaults = Object.freeze(
typeof PDFJSDev === "undefined"
? AppOptions.getAll(OptionKind.PREFERENCE, /* defaultOnly = */ true)
: PDFJSDev.eval("DEFAULT_PREFERENCES")
);

#prefs = Object.create(null);

#initializedPromise = null;

static #eventToDispatch = new Set(["toolbarDensity"]);
Expand All @@ -54,19 +46,13 @@ class BasePreferences {

this.#initializedPromise = this._readFromStorage(this.#defaults).then(
({ browserPrefs, prefs }) => {
const options = Object.create(null);

for (const [name, val] of Object.entries(this.#browserDefaults)) {
const prefVal = browserPrefs?.[name];
options[name] = typeof prefVal === typeof val ? prefVal : val;
if (
(typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
AppOptions._checkDisablePreferences()
) {
return;
}
for (const [name, val] of Object.entries(this.#defaults)) {
const prefVal = prefs?.[name];
// Ignore preferences whose types don't match the default values.
options[name] = this.#prefs[name] =
typeof prefVal === typeof val ? prefVal : val;
}
AppOptions.setAll(options, /* init = */ true);
AppOptions.setAll({ ...browserPrefs, ...prefs }, /* prefs = */ true);
}
);

Expand Down Expand Up @@ -103,23 +89,13 @@ class BasePreferences {
throw new Error("Not implemented: #updatePref");
}
await this.#initializedPromise;

if (name in this.#browserDefaults) {
if (typeof value !== typeof this.#browserDefaults[name]) {
return; // Invalid preference value.
}
} else if (name in this.#defaults) {
if (typeof value !== typeof this.#defaults[name]) {
return; // Invalid preference value.
}
this.#prefs[name] = value;
} else {
return; // Invalid preference.
}
AppOptions.set(name, value);
AppOptions.setAll({ [name]: value }, /* prefs = */ true);

if (BasePreferences.#eventToDispatch.has(name)) {
this.eventBus?.dispatch(name.toLowerCase(), { source: this, value });
this.eventBus?.dispatch(name.toLowerCase(), {
source: this,
value: AppOptions.get(name),
});
}
}

Expand All @@ -133,16 +109,9 @@ class BasePreferences {
throw new Error("Please use `about:config` to change preferences.");
}
await this.#initializedPromise;
const oldPrefs = structuredClone(this.#prefs);

this.#prefs = Object.create(null);
try {
await this._writeToStorage(this.#defaults);
} catch (reason) {
// Revert all preference values, since writing to storage failed.
this.#prefs = oldPrefs;
throw reason;
}
AppOptions.setAll(this.#defaults, /* prefs = */ true);

await this._writeToStorage(this.#defaults);
}

/**
Expand All @@ -157,37 +126,10 @@ class BasePreferences {
throw new Error("Please use `about:config` to change preferences.");
}
await this.#initializedPromise;
const defaultValue = this.#defaults[name],
oldPrefs = structuredClone(this.#prefs);

if (defaultValue === undefined) {
throw new Error(`Set preference: "${name}" is undefined.`);
} else if (value === undefined) {
throw new Error("Set preference: no value is specified.");
}
const valueType = typeof value,
defaultType = typeof defaultValue;

if (valueType !== defaultType) {
if (valueType === "number" && defaultType === "string") {
value = value.toString();
} else {
throw new Error(
`Set preference: "${value}" is a ${valueType}, expected a ${defaultType}.`
);
}
} else if (valueType === "number" && !Number.isInteger(value)) {
throw new Error(`Set preference: "${value}" must be an integer.`);
}
AppOptions.setAll({ [name]: value }, /* prefs = */ true);

this.#prefs[name] = value;
try {
await this._writeToStorage(this.#prefs);
} catch (reason) {
// Revert all preference values, since writing to storage failed.
this.#prefs = oldPrefs;
throw reason;
}
const prefs = AppOptions.getAll(OptionKind.PREFERENCE);
await this._writeToStorage(prefs);
}

/**
Expand All @@ -201,12 +143,7 @@ class BasePreferences {
throw new Error("Not implemented: get");
}
await this.#initializedPromise;
const defaultValue = this.#defaults[name];

if (defaultValue === undefined) {
throw new Error(`Get preference: "${name}" is undefined.`);
}
return this.#prefs[name] ?? defaultValue;
return AppOptions.get(name);
}

get initializedPromise() {
Expand Down

0 comments on commit e04ac83

Please sign in to comment.