- Notifications
You must be signed in to change notification settings - Fork1.8k
Fix Caps lock bug when using some IME on macOS#3728
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
serkodev commentedMay 13, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@Tyriar Please let me know if you have any comments on this as this bug is really annoying when using VSCode. |
Tyriar left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Won't this break non-US keyboard layouts that use characters beyond a-zA-Z?
serkodev commentedMay 13, 2022
I've tried Taiwan and Japanese layout keyboard and there is no problems. |
serkodev commentedMay 13, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@Tyriar If there are some other keyboard layouts or something else need me to help to test, just let me know. 😊 This bug has affected me for at least 6 years when I was new to web development with VSCode. |
serkodev commentedMay 13, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
If concern about if (event.key && !event.ctrlKey && !event.altKey && !event.metaKey && event.key.length === 1 && ( (event.keyCode >= 65 && event.keyCode <= 90) || (event.keyCode >= 97 && event.keyCode <= 122) )) { // Include only keys in A-Z/a-z. // HACK: Need process these key events in 'onkeypress' to fix a IME bug on macOS Chrome. fix for #2457- return true;+ const c = event.key.charCodeAt(0)+ if ((c >= 65 && c <= 90) || (c >= 97 && c <= 122)) {+ return true;+ }}
If you think this solution is acceptable, I can push a commit with this. |
serkodev commentedMay 24, 2022
@Tyriar May I know is there feed back? Thank you |
src/browser/Terminal.ts Outdated
| )){ | ||
| // Include only keys in A-Z/a-z. | ||
| // HACK: Need process these key events in 'onkeypress' to fix a IME bug on macOS Chrome. fix for #2457 | ||
| returntrue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Related to#3728 (comment), here is the review comment with suggestion change. Welcome to commit suggestion If you think this is a better solution.
| returntrue; | |
| constc=event.key.charCodeAt(0); | |
| if((c>=65&&c<=90)||(c>=97&&c<=122)){ | |
| returntrue; | |
| } |
serkodev commentedJun 7, 2022
@Tyriar@meganrogge World you please take a look on this? Hope to give some advice if possible. |
Tyriar left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@serkodev thanks a bunch for looking into this and sorry about the delay. I pushed a change to move off the deprecated keyCode and it still works great. This should land in the next version of VS Code 🙂
Uh oh!
There was an error while loading.Please reload this page.
Fixes#2457
Fixes#3155
The reason for this bug is that when Chrome uses some specific input methods and keydown, it will enter a key that is inconsistent with the actual one.
But in
onkeypress, it can return the correctkeyCode. Therefore, the solution to this problem is to only process characters (A-Za-z) duringonkeypress.In addition to the macOS built-in Pinyin input method proposed in the earlier issue, some other macOS third-party input methods such asOpenVanilla will encounter the same problem.
Fixesmicrosoft/vscode#109565
Fixesmicrosoft/vscode#69367
Fixesmicrosoft/vscode#138028