Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No link for disabled extensions on Workspace Trust screen #126614

Closed
ayatokura opened this issue Jun 17, 2021 · 44 comments
Closed

No link for disabled extensions on Workspace Trust screen #126614

ayatokura opened this issue Jun 17, 2021 · 44 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug L10N Translation issue verified Verification succeeded workspace-trust Trusted workspaces
Milestone

Comments

@ayatokura
Copy link

Issue Type: Bug

Extension links do not appear in the restricted mode screen of the Workspace Trust.

VS Code version: Code 1.57.0 (b4c1bd0, 2021-06-09T17:22:31.215Z)
OS version: Darwin x64 20.5.0
Restricted Mode: No

System Info
Item Value
CPUs Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz (8 x 2700)
GPU Status 2d_canvas: enabled
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
webgl: enabled
webgl2: enabled
Load (avg) 2, 2, 2
Memory (System) 16.00GB (0.10GB free)
Process Argv --crash-reporter-id 8c8f0ffc-f327-42cf-9387-d1086088a422
Screen Reader no
VM 0%
Extensions (71)
Extension Author (truncated) Version
vscode-browser-preview auc 0.7.1
vscode-twitter aus 0.8.3
releasenote-viewer aya 0.1.1
emojisense bie 0.8.0
markdown-checkbox bie 0.1.3
converttoasciiart Bit 1.0.3
npm-intellisense chr 1.3.1
path-intellisense chr 2.3.0
codeswing cod 0.0.16
bracket-pair-colorizer Coe 1.0.61
bracket-pair-colorizer-2 Coe 0.2.1
vscode-eslint dba 2.1.23
githistory don 0.6.16
gitlens eam 11.5.1
vscode-npm-script eg2 0.3.22
vscode-node-red for 0.2.0
vscode-pull-request-github Git 0.27.1
gc-excelviewer Gra 3.0.42
vscode-drawio hed 1.5.0
vscode-power-mode hoo 2.2.0
ibm-developer IBM 0.0.14
search-node-modules jas 1.3.0
custom-presentation-mode jds 0.0.1
vscode-peacock joh 3.9.1
chat kar 0.35.0
thinking-in-code los 0.0.7
marp-vscode mar 1.0.2
vscode-docker ms- 1.13.0
vscode-language-pack-ja MS- 1.57.1
csharp ms- 1.23.12
vscode-edge-devtools ms- 1.1.9
vscode-kubernetes-tools ms- 1.3.3
python ms- 2021.6.944021595
vscode-pylance ms- 2021.6.2
jupyter ms- 2021.6.999406279
remote-containers ms- 0.183.0
remote-ssh ms- 0.65.7
remote-ssh-edit ms- 0.65.7
vscode-remote-extensionpack ms- 0.21.0
vscode-github-issue-notebooks ms- 0.0.102
vscode-typescript-tslint-plugin ms- 1.3.3
vsliveshare ms- 1.0.4419
vsliveshare-audio ms- 0.1.91
vsliveshare-pack ms- 0.4.0
vsonline ms- 1.0.3076
debugger-for-chrome msj 4.12.12
debugger-for-edge msj 1.0.15
vscode-paste-image mus 1.0.4
zenn-editor neg 0.7.2
javascript-test-runner osh 1.2.4
project-initializer red 0.1.0
vscode-commons red 0.0.6
vscode-openshift-connector red 0.2.7
vscode-openshift-extension-pack red 0.0.2
vscode-rsp-ui red 0.23.9
vscode-server-connector red 0.23.11
vscode-yaml red 0.20.0
LiveServer rit 5.6.1
synthwave-vscode Rob 0.1.8
html-preview-vscode tht 0.2.5
vscode-pets ton 1.8.0
luna-paint Tyr 0.6.0
codetour vsl 0.0.56
gistfs vsl 0.2.9
gitdoc vsl 0.0.8
spaces vsl 0.1.2
nodejs-extension-pack wad 0.1.9
JavaScriptSnippets xab 1.8.0
directory-configuration-generator yar 0.0.4
markdown-pdf yza 1.4.4
markdown-all-in-one yzh 3.4.0

(5 theme extensions excluded)

A/B Experiments
vsliv368cf:30146710
vsreu685:30147344
python383cf:30185419
pythonvspyt602:30300191
vspor879:30202332
vspor708:30202333
vspor363:30204092
pythonvspyt639:30300192
pythontb:30283811
pythonvspyt551:30311712
vspre833:30321513
pythonptprofiler:30281270
vshan820:30294714
pythondataviewer:30285071
vscus158:30321503
pythonvsuse255cf:30319632
vscorehov:30309549
vscod805:30301674
binariesv615:30321888

@vscodebot
Copy link

vscodebot bot commented Jun 17, 2021

@ayatokura
Copy link
Author

Here is a screenshot. Thanks.
VSCode_Trust1

@sandy081 sandy081 assigned lszomoru and unassigned sandy081 Jun 18, 2021
@lszomoru lszomoru added the workspace-trust Trusted workspaces label Jun 21, 2021
@lszomoru lszomoru added this to the June 2021 milestone Jun 21, 2021
@lszomoru lszomoru added the bug Issue identified by VS Code Team member as probable bug label Jun 21, 2021
@lszomoru
Copy link
Member

@TylerLeonhardt, any insights on this? I thought that this has already been fixed.

@TylerLeonhardt
Copy link
Member

@lszomoru this is because in the translation they messed up the markdown syntax and added a space between the [] and ().
image

I suggest that you do the following that @JacksonKearl did here:
https://github.com/microsoft/vscode/pull/125790/files#diff-8d21c4b4e58c010a4d4fa747e289dba2db97c90f78f576e720e904ea0cef29f4R121

and have the markdown syntax be included in the interpolated bit and not be passed to the translators.

@sbatten
Copy link
Member

sbatten commented Jun 21, 2021

@TylerLeonhardt I don't know if its actually safe to do that. The markdown bit is actually part of the phrase here and some languages might need to reorder the text. This is why I haven't done it this way currently.

@TylerLeonhardt
Copy link
Member

Good point. Can you at least include a comment to tell the translators to honor the []() syntax? Here's an example of a comment:
https://github.com/microsoft/vscode/blob/HEAD/src/vs/workbench/contrib/debug/browser/welcomeView.ts#L125-L126

@JacksonKearl
Copy link
Contributor

JacksonKearl commented Jun 21, 2021

@sbatten I can't imagine translating "[{0} extensions]({1}) are disabled or have limited functionality" would provide more context than "{0} are disabled or have limited functionality" and "{0} extensions". Linguistically, the hole is just a plural noun... if it were a phrase or fragment I agree it'd be a concern.

I don't think it's safe to expect the translators to know and persist MD syntax, and while the comments help it still feels risky (the above comment makes no mention of anything special besides command, and could be subject to the same bug as here)

@sbatten
Copy link
Member

sbatten commented Jun 21, 2021

well, it's a count and plural noun, and as someone who is not an expert on linguistics of the world, I didn't want to assume that its safe. specifically, because a broken translation with markdown syntax is a fairly easy one-time fix, whereas the alternative potentially limits the end result.

I am happy to change it if this is not a concern

@JacksonKearl
Copy link
Contributor

JacksonKearl commented Jun 21, 2021

On one had you have a broken experience if translators make a mistake in MD syntax (even with comment fields there's room for human error), on the other the translated string might be awkward but the experience is more-or-less guaranteed to work.

🤷‍♀️

One more consideration is that we don't actually classify the holes at all. Translators would have no idea if the hole is a count or an adjective or an interjection or what.

@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Jun 23, 2021

I'll leave it up to you @sbatten & @lszomoru but I think your options are:

  • Add a comment for the translators to not mess up []() - could break the link if the translator isn't reliable but will be grammatically correct
  • Have the inside part of the button be a different string - might not be always grammatically correct but the link will always work

I'm kinda in favor of the 2nd option... personally but don't care which route you take.

@TylerLeonhardt TylerLeonhardt removed their assignment Jun 23, 2021
@sbatten
Copy link
Member

sbatten commented Jun 24, 2021

I ended up encountering a similar structured string elsewhere in the same file. In that case, using the second approach would have resulted in a translator having 3 strings to translate without context that they would be concatenated. For that reason and consistency, I've opted for a comment.

@mjbvz mjbvz added the author-verification-requested Issues potentially verifiable by issue author label Jul 1, 2021
@rzhao271
Copy link
Contributor

rzhao271 commented Jul 1, 2021

The translation still seems broken in French, though I'm not sure how long it takes for the translations to go in.

@rzhao271 rzhao271 reopened this Jul 1, 2021
@rzhao271 rzhao271 added the verification-found Issue verification failed label Jul 1, 2021
@TylerLeonhardt
Copy link
Member

We may have to wait for the next release of this language pack (next Tuesday)

@sbatten sbatten removed the verification-found Issue verification failed label Jul 2, 2021
@sbatten sbatten closed this as completed Jul 2, 2021
@sbatten
Copy link
Member

sbatten commented Jul 2, 2021

Verification needs the latest language pack... Closing this item to keep it off the endgame query for open issues.

@rzhao271
Copy link
Contributor

The problem seems to persist in v1.60.1 for at least the French language pack

Screenshot

@rzhao271 rzhao271 reopened this Aug 25, 2021
@rzhao271 rzhao271 added the verification-found Issue verification failed label Aug 25, 2021
@TylerLeonhardt
Copy link
Member

@sbatten @lszomoru until I have time to actually work on the {text, link} syntax that Dirk suggested (which I think will take time considering I have to make changes in a few places... @dbaeumer I would love any pointers for that) I'd suggest fixing this one case either using a string replace to get rid of the middle spaces or somehow exclude the link syntax from getting sent to the translators by including it in the arguments of localize.

@TylerLeonhardt
Copy link
Member

@v-mholloway where are we going wrong here on the translation side? There's clearly a comment in Resource Fabric:
image

Please inform the translators that they absolutely need to read the comments otherwise they will break the product.

@dbaeumer
Copy link
Member

@TylerLeonhardt it is the same place as describe here: microsoft/vscode-loader#31

@lszomoru lszomoru modified the milestones: August 2021, September 2021 Aug 27, 2021
sbatten added a commit that referenced this issue Oct 22, 2021
@sbatten
Copy link
Member

sbatten commented Oct 22, 2021

I did a basic implementation for the workspace trust translations where I remove the whitespace in the middle of a markdown link. I did this in favor of introducing yet more localized strings and changing them again for all translations.

@sbatten sbatten removed the verification-found Issue verification failed label Oct 22, 2021
@danyeh
Copy link
Contributor

danyeh commented Oct 22, 2021

@cristianosuzuki77, for information, need to seek the way to enforce no space added in translation for markdown link between [Link text] and (Link).

@TylerLeonhardt
Copy link
Member

Ok since we have @sbatten's implementation and microsoft/vscode-loader#31 let's go ahead and close this.

@danyeh I'm curious to know what is being done to ensure there is no space added between the [Link text] and (Link) on your end for our own context.

@TylerLeonhardt
Copy link
Member

verification steps:

  • Using the Japanese or French language pack
  • Open the Workspace Trust editor

Ensure buttons are correctly rendered.

@danyeh
Copy link
Contributor

danyeh commented Nov 23, 2021

@TylerLeonhardt , add a dev comment like {Locked="]({1})"} should be able to enforce there is no space added between.

  • There will validation error if space added. (previous plane text comment won't cause validation error).
  • Assume translation violate the rule still got checked in. Build process will revert the translation back to English because violate the loc rule. Hence there won't be function break.

@TylerLeonhardt
Copy link
Member

@danyeh just to clarify... when you say "dev comment" you mean like the comments that show up here?
image

so if I add a comment that says {Locked="]({1})"} that will throw a validation error if there's a space?

@danyeh
Copy link
Contributor

danyeh commented Nov 24, 2021

@TylerLeonhardt , yes, what you mentioned is correct.

@aeschli
Copy link
Contributor

aeschli commented Dec 2, 2021

Verified in
Version : 1.63.0-insider (user setup)
Validation : 68a1e2f
Date : 2021-12-02T05:15:40.605Z
Electron : 13.5.2
Chromium: 91.0.4472.164
Node.js : 14.16.0
V8 : 9.1.269.39-electron.0
Système d’exploitation : Windows_NT x64 10.0.19044
image

@aeschli aeschli added the verified Verification succeeded label Dec 2, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Dec 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug L10N Translation issue verified Verification succeeded workspace-trust Trusted workspaces
Projects
None yet
Development

No branches or pull requests

14 participants