From ea97c960cb5779f6e913835e357bf37ffda34cb6 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 d087ede365..c2f58b7bb4 100644 --- a/chrome/content/zotero/xpcom/fileHandlers.js +++ b/chrome/content/zotero/xpcom/fileHandlers.js @@ -157,9 +157,6 @@ Zotero.FileHandlers = { catch (e) { Zotero.logError(e); } - finally { - Zotero.Utilities.Internal.Environment.restoreMozillaVariables(); - } } try { @@ -508,9 +505,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 76fb99efa8..15048c6547 100644 --- a/chrome/content/zotero/xpcom/utilities_internal.js +++ b/chrome/content/zotero/xpcom/utilities_internal.js @@ -1897,6 +1897,7 @@ Zotero.Utilities.Internal = { var startup = Services.startup; if (restart) { Zotero.restarting = true; + this.Environment.restoreMozillaVariables(); } startup.quit(startup.eAttemptQuit | (restart ? startup.eRestart : 0)); }, @@ -2999,21 +3000,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 e110bbac76..650505c364 100644 --- a/chrome/content/zotero/xpcom/zotero.js +++ b/chrome/content/zotero/xpcom/zotero.js @@ -1042,9 +1042,6 @@ Services.scriptloader.loadSubScript("resource://zotero/polyfill.js"); ); } } - finally { - Zotero.Utilities.Internal.Environment.restoreMozillaVariables(); - } }; @@ -1072,9 +1069,7 @@ Services.scriptloader.loadSubScript("resource://zotero/polyfill.js"); 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); }; @@ -1104,27 +1099,18 @@ Services.scriptloader.loadSubScript("resource://zotero/polyfill.js"); 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); @@ -1151,10 +1137,7 @@ Services.scriptloader.loadSubScript("resource://zotero/polyfill.js"); + "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); @@ -1163,11 +1146,6 @@ Services.scriptloader.loadSubScript("resource://zotero/polyfill.js"); var args = [url]; proc.runw(false, args, args.length); } - finally { - if (mozCleared) { - Zotero.Utilities.Internal.Environment.restoreMozillaVariables(); - } - } }