fix: add nlsConfig to webWorker #192808 #199258

yiliang114 · 2023-11-28T03:35:43Z

After modification:

Fixes #192808

TylerLeonhardt · 2023-11-30T17:07:05Z

src/vs/base/worker/workerMain.ts

+		const nlsConfig: any = {};
+		nlsConfig['vs/nls'] = {
+			availableLanguages: {
+				'*': navigator.language.toLowerCase()


I think we want to use a similar change as here:

vscode/src/vs/code/browser/workbench/workbench.html

Lines 46 to 56 in cf1b0ce

// Normalize locale to lowercase because translationServiceUrl is case-sensitive.

// ref: https://github.com/microsoft/vscode/issues/187795

const locale = localStorage.getItem('vscode.nls.locale') || navigator.language.toLowerCase();

if (!locale.startsWith('en')) {

nlsConfig['vs/nls'] = {

availableLanguages: {

'*': locale

},

translationServiceUrl: '{{WORKBENCH_NLS_BASE_URL}}'

};

}

where we look at local storage as well so that if the user has configured a display language different than their browser that still works.

One thing I'm curious about is if this works in Desktop. I would guess that electron does all the hard work of plumbing the language to navigator.language but it would be good to confirm.

Yes, I have also observed the need to read the localstorge before. However, there is a problem. Similar to vscode.dev, the domain name is inconsistent with the domain where the iframe is located. Or we need to let the main process tell the iframe after it gets the localstorge, and then tell the worker.

At the same time, I will try to see the electron treatment.

yiliang114 · 2023-12-01T09:55:12Z

src/vs/workbench/services/extensions/worker/webWorkerExtensionHostIframe.html

@@ -73,6 +74,10 @@
 			}

 			const worker = new Worker(workerUrl, { name });
+			if (locale) {
+				// send before `vs/workbench/api/worker/extensionHostWorker`load event.
+				worker.postMessage(`worker:init-locale:${locale}`);


Because the worker cannot read the localstorge directly and the iframe may not be in the same domain name as the main application, the method used is to obtain the language in the host, pass it to the iframe, and then pass it to the worker.

I don't know if this will be strange

TylerLeonhardt · 2023-12-06T01:47:03Z

src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts


 		const iframeModulePath = 'vs/workbench/services/extensions/worker/webWorkerExtensionHostIframe.html';
+		const locale = localStorage.getItem('vscode.nls.locale') || navigator.language.toLowerCase();


Since this is running on the main side, I think you can just use language or Language.value():

vscode/src/vs/base/common/platform.ts

Lines 182 to 188 in 7a104d4

export const language = _language;

export namespace Language {

export function value(): string {

return language;

}

the implementation of this will grab the local storage value or navigator value.

…avigator

…kly-earthworm

TylerLeonhardt · 2023-12-06T04:48:08Z

src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts

@@ -109,12 +110,19 @@ export class WebWorkerExtensionHost extends Disposable implements IExtensionHost
 				const res = new URL(`${baseUrl}/out/${iframeModulePath}${suffix}`);
 				res.searchParams.set('parentOrigin', mainWindow.origin);
 				res.searchParams.set('salt', stableOriginUUID);
+				if (locale !== 'en') {


Sorry, also in platform there is a LANGUAGE_DEFAULT which you can use instead of hardcoding 'en'

TylerLeonhardt · 2023-12-06T04:48:19Z

src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts

 				return res.toString();
 			}

 			console.warn(`The web worker extension host is started in a same-origin iframe!`);
 		}

+		if (locale !== 'en') {


TylerLeonhardt · 2023-12-06T04:51:09Z

I added @alexdima to this PR since he has a better understanding of our AMD & webworker code. I'd like a review from him before this goes in

yiliang114 · 2023-12-06T05:13:55Z

I added @alexdima to this PR since he has a better understanding of our AMD & webworker code. I'd like a review from him before this goes in

okey!

TylerLeonhardt · 2023-12-06T20:32:30Z

src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts

@@ -83,9 +83,10 @@ export class WebWorkerExtensionHost extends Disposable implements IExtensionHost
 		}
 		COI.addSearchParam(suffixSearchParams, true, true);

-		const suffix = `?${suffixSearchParams.toString()}`;
+		let suffix = `?${suffixSearchParams.toString()}`;


I feel like you could just do this:

Suggested change

let suffix = `?${suffixSearchParams.toString()}`;

if (!platform.Language.isDefaultVariant()) {

suffixSearchParams.set('locale', platform.language);

}

const suffix = `?${suffixSearchParams.toString()}`;

and get rid of the other changes in this file.

vivodi · 2024-12-29T17:11:08Z

Is this still relevant?

fix: add nlsConfig to webWorker microsoft#192808

1f40cff

vscodenpa assigned mjbvz Nov 28, 2023

mjbvz assigned TylerLeonhardt and unassigned mjbvz Nov 28, 2023

TylerLeonhardt reviewed Nov 30, 2023

View reviewed changes

yiliang114 added 2 commits December 1, 2023 17:46

fix: add nlsConfig to webWorker microsoft#192808

25de25f

Merge remote-tracking branch 'origin/main' into prickly-earthworm

01f5819

yiliang114 force-pushed the prickly-earthworm branch from 38e7cd1 to 01f5819 Compare December 1, 2023 09:51

Merge branch 'main' into prickly-earthworm

8faf47c

yiliang114 commented Dec 1, 2023

View reviewed changes

Merge branch 'main' into prickly-earthworm

0167ea5

TylerLeonhardt reviewed Dec 6, 2023

View reviewed changes

TylerLeonhardt requested a review from alexdima December 6, 2023 01:48

yiliang114 added 3 commits December 6, 2023 10:22

Merge remote-tracking branch 'origin/main' into prickly-earthworm

ee0f46f

fix: use platform.language replace get value from localStorage or n…

58dabe3

…avigator

Merge remote-tracking branch 'yiliang114/prickly-earthworm' into pric…

a88dc1a

…kly-earthworm

TylerLeonhardt reviewed Dec 6, 2023

View reviewed changes

Use platform.LANGUAGE_DEFAULT to replace 'en'

7b5f677

TylerLeonhardt reviewed Dec 6, 2023

View reviewed changes

yiliang114 added 9 commits December 8, 2023 14:58

Merge remote-tracking branch 'origin/main' into prickly-earthworm

c91d6fa

Fix microsoft#192808

415cbe0

Merge branch 'main' into prickly-earthworm

5b99eba

Merge branch 'main' into prickly-earthworm

ad14cc9

Merge branch 'main' into prickly-earthworm

28b02c2

Merge branch 'main' into prickly-earthworm

e7c58a3

Merge branch 'main' into prickly-earthworm

0048b55

Merge branch 'main' into prickly-earthworm

4f221a3

Merge branch 'main' into prickly-earthworm

ab58c3a

alexdima self-assigned this Mar 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add nlsConfig to webWorker #192808 #199258

fix: add nlsConfig to webWorker #192808 #199258

yiliang114 commented Nov 28, 2023 •

edited by TylerLeonhardt

Loading

TylerLeonhardt Nov 30, 2023

TylerLeonhardt Nov 30, 2023

yiliang114 Dec 1, 2023

yiliang114 Dec 1, 2023

TylerLeonhardt Dec 6, 2023

TylerLeonhardt Dec 6, 2023

TylerLeonhardt Dec 6, 2023

TylerLeonhardt commented Dec 6, 2023

yiliang114 commented Dec 6, 2023

TylerLeonhardt Dec 6, 2023 •

edited

Loading

vivodi commented Dec 29, 2024

	// Normalize locale to lowercase because translationServiceUrl is case-sensitive.
	// ref: https://github.com/microsoft/vscode/issues/187795
	const locale = localStorage.getItem('vscode.nls.locale') \|\| navigator.language.toLowerCase();
	if (!locale.startsWith('en')) {
	nlsConfig['vs/nls'] = {
	availableLanguages: {
	'*': locale
	},
	translationServiceUrl: '{{WORKBENCH_NLS_BASE_URL}}'
	};
	}


		const iframeModulePath = 'vs/workbench/services/extensions/worker/webWorkerExtensionHostIframe.html';
		const locale = localStorage.getItem('vscode.nls.locale') \|\| navigator.language.toLowerCase();

	export const language = _language;

	export namespace Language {

	export function value(): string {
	return language;
	}

fix: add nlsConfig to webWorker #192808 #199258

Are you sure you want to change the base?

fix: add nlsConfig to webWorker #192808 #199258

Conversation

yiliang114 commented Nov 28, 2023 • edited by TylerLeonhardt Loading

TylerLeonhardt Nov 30, 2023

Choose a reason for hiding this comment

TylerLeonhardt Nov 30, 2023

Choose a reason for hiding this comment

yiliang114 Dec 1, 2023

Choose a reason for hiding this comment

yiliang114 Dec 1, 2023

Choose a reason for hiding this comment

TylerLeonhardt Dec 6, 2023

Choose a reason for hiding this comment

TylerLeonhardt Dec 6, 2023

Choose a reason for hiding this comment

TylerLeonhardt Dec 6, 2023

Choose a reason for hiding this comment

TylerLeonhardt commented Dec 6, 2023

yiliang114 commented Dec 6, 2023

TylerLeonhardt Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

vivodi commented Dec 29, 2024

yiliang114 commented Nov 28, 2023 •

edited by TylerLeonhardt

Loading

TylerLeonhardt Dec 6, 2023 •

edited

Loading