From 96dd7b16acf74bd6d71771126f280a7c9bde2cc3 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 8 Aug 2025 02:48:58 -0400 Subject: [PATCH] Better method of unsetting and restoring Mozilla environment variables Hopefully a better fix for #4981, which wasn't working properly on at least some Linux systems because the variables were getting restored before the subprocess launched. This delays a (debounced) second, to give the subprocess time to start, and then automatically restores the variables. An explicit restart via app code also immediately restores the variables, since restarts on Linux inherent the environment and don't use our startup script. (An upgrade restart could still happen when the variables were cleared, but you'd have to be extremely unlucky -- launching URLs or files while also performing a manual restart the same second.) --- chrome/content/zotero/xpcom/fileHandlers.js | 7 +--- .../zotero/xpcom/utilities_internal.js | 32 ++++++++++++++--- chrome/content/zotero/xpcom/zotero.js | 34 ++++--------------- 3 files changed, 35 insertions(+), 38 deletions(-) diff --git a/chrome/content/zotero/xpcom/fileHandlers.js b/chrome/content/zotero/xpcom/fileHandlers.js index a1fa4b1765..5a9bc2834b 100644 --- a/chrome/content/zotero/xpcom/fileHandlers.js +++ b/chrome/content/zotero/xpcom/fileHandlers.js @@ -171,9 +171,6 @@ Zotero.FileHandlers = { catch (e) { Zotero.logError(e); } - finally { - Zotero.Utilities.Internal.Environment.restoreMozillaVariables(); - } } try { @@ -522,9 +519,7 @@ Zotero.FileHandlers = { Zotero.Utilities.Internal.Environment.clearMozillaVariables(); // Do not await - var promise = Zotero.Utilities.Internal.exec(command, args); - - promise.finally(() => Zotero.Utilities.Internal.Environment.restoreMozillaVariables()); + Zotero.Utilities.Internal.exec(command, args); }, }; diff --git a/chrome/content/zotero/xpcom/utilities_internal.js b/chrome/content/zotero/xpcom/utilities_internal.js index abc44033e6..35545f588d 100644 --- a/chrome/content/zotero/xpcom/utilities_internal.js +++ b/chrome/content/zotero/xpcom/utilities_internal.js @@ -1929,6 +1929,7 @@ Zotero.Utilities.Internal = { var startup = Services.startup; if (restart) { Zotero.restarting = true; + this.Environment.restoreMozillaVariables(); } startup.quit(startup.eAttemptQuit | (restart ? startup.eRestart : 0)); }, @@ -3066,21 +3067,44 @@ Zotero.Utilities.Internal.Environment = { * using the wrong Firefox profile when launching URLs or PDFs. On Windows, it's not necessary * to call this when launching URLs, only processes. * + * The variables are restored a (debounced) second after this is called. Restoring mostly isn't + * necessary, since most new launches of Zotero would use the modified launcher, but a restart + * on Linux (e.g., during an upgrade) skips our shell script where we set these variables. + * * https://github.com/zotero/zotero/issues/4981 */ clearMozillaVariables: function () { + const RESTORE_DEBOUNCE_DELAY = 1000; + this.unset("MOZ_ALLOW_DOWNGRADE"); this.unset("MOZ_LEGACY_PROFILES"); + + if (this._restoreTimeout) { + clearTimeout(this._restoreTimeout); + delete this._restoreTimeout; + } + + this._restoreTimeout = setTimeout(() => { + delete this._restoreTimeout; + this._restoreMozillaVariables(); + }, RESTORE_DEBOUNCE_DELAY); }, /** - * Re-set the Mozilla environment variables that we changed in the launcher + * Immediately restore the Mozilla environment variables * - * Call this in a finally() after using unsetMozillaVariables(). This mostly isn't necessary, - * since most new launches of Zotero would use the modified launcher, but a restart on Linux - * skips our shell script where we set these variables. + * The variables are automatically restored shortly after they're cleared, but this can be + * called before explicit restarts. */ restoreMozillaVariables: function () { + if (this._restoreTimeout) { + clearTimeout(this._restoreTimeout); + delete this._restoreTimeout; + } + this._restoreMozillaVariables(); + }, + + _restoreMozillaVariables: function () { var env = Cc["@mozilla.org/process/environment;1"].getService(Ci.nsIEnvironment); env.set("MOZ_ALLOW_DOWNGRADE", "1"); env.set("MOZ_LEGACY_PROFILES", "1"); diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js index 0e68211fa7..9776f8e33e 100644 --- a/chrome/content/zotero/xpcom/zotero.js +++ b/chrome/content/zotero/xpcom/zotero.js @@ -1032,9 +1032,6 @@ const { CommandLineOptions } = ChromeUtils.importESModule("chrome://zotero/conte ); } } - finally { - Zotero.Utilities.Internal.Environment.restoreMozillaVariables(); - } }; @@ -1062,9 +1059,7 @@ const { CommandLineOptions } = ChromeUtils.importESModule("chrome://zotero/conte Zotero.Utilities.Internal.Environment.clearMozillaVariables(); // Async, but we don't want to block - var promise = Zotero.Utilities.Internal.exec(applicationPath, args); - - promise.finally(() => Zotero.Utilities.Internal.Environment.restoreMozillaVariables()); + Zotero.Utilities.Internal.exec(applicationPath, args); }; @@ -1094,27 +1089,18 @@ const { CommandLineOptions } = ChromeUtils.importESModule("chrome://zotero/conte if (!found.value) { throw new Error(`Handler not found for '${scheme}' URLs`); } - try { - if (!Zotero.isWin) { - Zotero.Utilities.Internal.Environment.clearMozillaVariables(); - } - - svc.loadURI(Services.io.newURI(url, null, null)); - } - finally { - if (!Zotero.isWin) { - Zotero.Utilities.Internal.Environment.restoreMozillaVariables(); - } + if (!Zotero.isWin) { + Zotero.Utilities.Internal.Environment.clearMozillaVariables(); } + + svc.loadURI(Services.io.newURI(url, null, null)); return; } } - var mozCleared = false; try { if (!Zotero.isWin) { Zotero.Utilities.Internal.Environment.clearMozillaVariables(); - mozCleared = true; } var uri = Services.io.newURI(url, null, null); @@ -1141,10 +1127,7 @@ const { CommandLineOptions } = ChromeUtils.importESModule("chrome://zotero/conte + "check extensions.zotero." + pref + " in about:config"); } - if (!mozCleared) { - Zotero.Utilities.Internal.Environment.clearMozillaVariables(); - mozCleared = true; - } + Zotero.Utilities.Internal.Environment.clearMozillaVariables(); var proc = Components.classes["@mozilla.org/process/util;1"] .createInstance(Components.interfaces.nsIProcess); @@ -1153,11 +1136,6 @@ const { CommandLineOptions } = ChromeUtils.importESModule("chrome://zotero/conte var args = [url]; proc.runw(false, args, args.length); } - finally { - if (mozCleared) { - Zotero.Utilities.Internal.Environment.restoreMozillaVariables(); - } - } }