Skip to content

Commit

Permalink
fix: LC_ALL env should not be changed (#26550)
Browse files Browse the repository at this point in the history
Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
  • Loading branch information
trop[bot] and zcbenz committed Nov 18, 2020
1 parent 9900e69 commit 570f982
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 12 deletions.
26 changes: 19 additions & 7 deletions shell/browser/electron_browser_main_parts.cc
Expand Up @@ -11,6 +11,7 @@
#include "base/base_switches.h"
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/optional.h"
#include "base/path_service.h"
#include "base/run_loop.h"
#include "base/strings/string_number_conversions.h"
Expand Down Expand Up @@ -369,23 +370,34 @@ int ElectronBrowserMainParts::PreCreateThreads() {
// which keys off of getenv("LC_ALL").
// We must set this env first to make ui::ResourceBundle accept the custom
// locale.
g_setenv("LC_ALL", locale.c_str(), TRUE);
std::unique_ptr<base::Environment> env(base::Environment::Create());
base::Optional<std::string> lc_all;
if (!locale.empty()) {
std::string str;
if (env->GetVar("LC_ALL", &str))
lc_all.emplace(std::move(str));
env->SetVar("LC_ALL", locale.c_str());
}
#endif

// Load resources bundle according to locale.
std::string loaded_locale = LoadResourceBundle(locale);

#if defined(OS_LINUX)
// Reset to the loaded locale if the custom locale is invalid.
if (loaded_locale != locale)
g_setenv("LC_ALL", loaded_locale.c_str(), TRUE);
#endif

// Initialize the app locale.
std::string app_locale = l10n_util::GetApplicationLocale(loaded_locale);
ElectronBrowserClient::SetApplicationLocale(app_locale);
fake_browser_process_->SetApplicationLocale(app_locale);

#if defined(OS_LINUX)
// Reset to the original LC_ALL since we should not be changing it.
if (!locale.empty()) {
if (lc_all)
env->SetVar("LC_ALL", *lc_all);
else
env->UnSetVar("LC_ALL");
}
#endif

// Force MediaCaptureDevicesDispatcher to be created on UI thread.
MediaCaptureDevicesDispatcher::GetInstance();

Expand Down
18 changes: 15 additions & 3 deletions spec-main/chromium-spec.ts
Expand Up @@ -299,10 +299,13 @@ describe('command line switches', () => {
});
describe('--lang switch', () => {
const currentLocale = app.getLocale();
const testLocale = async (locale: string, result: string) => {
const testLocale = async (locale: string, result: string, printEnv: boolean = false) => {
const appPath = path.join(fixturesPath, 'api', 'locale-check');
const electronPath = process.execPath;
appProcess = ChildProcess.spawn(electronPath, [appPath, `--set-lang=${locale}`]);
const args = [appPath, `--set-lang=${locale}`];
if (printEnv) {
args.push('--print-env');
}
appProcess = ChildProcess.spawn(process.execPath, args);

let output = '';
appProcess.stdout.on('data', (data) => { output += data; });
Expand All @@ -314,6 +317,15 @@ describe('command line switches', () => {

it('should set the locale', async () => testLocale('fr', 'fr'));
it('should not set an invalid locale', async () => testLocale('asdfkl', currentLocale));

const lcAll = String(process.env.LC_ALL);
ifit(process.platform === 'linux')('current process has a valid LC_ALL env', async () => {
// The LC_ALL env should not be set to DOM locale string.
expect(lcAll).to.not.equal(app.getLocale());
});
ifit(process.platform === 'linux')('should not change LC_ALL', async () => testLocale('fr', lcAll, true));
ifit(process.platform === 'linux')('should not change LC_ALL when setting invalid locale', async () => testLocale('asdfkl', lcAll, true));
ifit(process.platform === 'linux')('should not change LC_ALL when --lang is not set', async () => testLocale('', lcAll, true));
});

describe('--remote-debugging-pipe switch', () => {
Expand Down
10 changes: 8 additions & 2 deletions spec/fixtures/api/locale-check/main.js
@@ -1,10 +1,16 @@
const { app } = require('electron');

const locale = process.argv[2].substr(11);
app.commandLine.appendSwitch('lang', locale);
if (locale.length !== 0) {
app.commandLine.appendSwitch('lang', locale);
}

app.whenReady().then(() => {
process.stdout.write(app.getLocale());
if (process.argv[3] === '--print-env') {
process.stdout.write(String(process.env.LC_ALL));
} else {
process.stdout.write(app.getLocale());
}
process.stdout.end();

app.quit();
Expand Down

0 comments on commit 570f982

Please sign in to comment.