fix: add nlsConfig to webWorker #192808 #199258
Conversation
src/vs/base/worker/workerMain.ts
Outdated
| const nlsConfig: any = {}; | ||
| nlsConfig['vs/nls'] = { | ||
| availableLanguages: { | ||
| '*': navigator.language.toLowerCase() |
There was a problem hiding this comment.
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
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
38e7cd1 to
01f5819
Compare
| const worker = new Worker(workerUrl, { name }); | ||
| if (locale) { | ||
| // send before `vs/workbench/api/worker/extensionHostWorker`load event. | ||
| worker.postMessage(`worker:init-locale:${locale}`); |
There was a problem hiding this comment.
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
| let suffix = `?${suffixSearchParams.toString()}`; | ||
|
|
||
| const iframeModulePath = 'vs/workbench/services/extensions/worker/webWorkerExtensionHostIframe.html'; | ||
| const locale = localStorage.getItem('vscode.nls.locale') || navigator.language.toLowerCase(); |
There was a problem hiding this comment.
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
the implementation of this will grab the local storage value or navigator value.
| const res = new URL(`${baseUrl}/out/${iframeModulePath}${suffix}`); | ||
| res.searchParams.set('parentOrigin', mainWindow.origin); | ||
| res.searchParams.set('salt', stableOriginUUID); | ||
| if (locale !== 'en') { |
There was a problem hiding this comment.
Sorry, also in platform there is a LANGUAGE_DEFAULT which you can use instead of hardcoding 'en'
| console.warn(`The web worker extension host is started in a same-origin iframe!`); | ||
| } | ||
|
|
||
| if (locale !== 'en') { |
|
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! |
| COI.addSearchParam(suffixSearchParams, true, true); | ||
|
|
||
| const suffix = `?${suffixSearchParams.toString()}`; | ||
| let suffix = `?${suffixSearchParams.toString()}`; |
There was a problem hiding this comment.
I feel like you could just do this:
| 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.
|
Is this still relevant? |
|
This works in the meantime thanks to the ESM work: 
|
After modification:
Fixes #192808