Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
Tyriar merged 10 commits intoxtermjs:masterfromserkodev:fix-caps-lock-ime
Jun 26, 2022

Conversation

@serkodev
Copy link
Contributor

@serkodevserkodev commentedApr 6, 2022
edited
Loading

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 inonkeypress, 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

1aron reacted with laugh emojikxxoling reacted with heart emojikxxoling reacted with rocket emojikxxoling reacted with eyes emoji
@serkodevserkodev changed the titleFix Caps lock bug on macOS IMEFix Caps lock bug when using some IME on macOSApr 7, 2022
@serkodev
Copy link
ContributorAuthor

serkodev commentedMay 13, 2022
edited
Loading

@Tyriar Please let me know if you have any comments on this as this bug is really annoying when using VSCode.
@meganrogge I would appreciate it if you could help to review too.

Copy link
Member

@TyriarTyriar left a 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
Copy link
ContributorAuthor

I've tried Taiwan and Japanese layout keyboard and there is no problems.

@serkodev
Copy link
ContributorAuthor

serkodev commentedMay 13, 2022
edited
Loading

@Tyriar If there are some other keyboard layouts or something else need me to help to test, just let me know. 😊
And welcome to discuss if there is other way to fix this bug.

This bug has affected me for at least 6 years when I was new to web development with VSCode.

@serkodev
Copy link
ContributorAuthor

serkodev commentedMay 13, 2022
edited
Loading

If concern aboutevent.key is non-English char, I think it can also add a condition to confirm theevent.key isa-zA-Z before returning.

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;+   }}

I did a benchmark test on usingevent.key.charCodeAt(0) and Regex/[a-z]/i.test(event.key), the result is usingcharCodeAt is 91.65% faster then Regex.

If you think this solution is acceptable, I can push a commit with this.

@serkodev
Copy link
ContributorAuthor

@Tyriar May I know is there feed back? Thank you

)){
// 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;
Copy link
ContributorAuthor

@serkodevserkodevMay 24, 2022
edited
Loading

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.

Suggested change
returntrue;
constc=event.key.charCodeAt(0);
if((c>=65&&c<=90)||(c>=97&&c<=122)){
returntrue;
}

@serkodev
Copy link
ContributorAuthor

@Tyriar@meganrogge World you please take a look on this? Hope to give some advice if possible.
This bug is still bothering me a lot.

@TyriarTyriar added this to the4.19.0 milestoneJun 26, 2022
Copy link
Member

@TyriarTyriar left a 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 🙂

serkodev and gqbre reacted with thumbs up emoji
@TyriarTyriarenabled auto-mergeJune 26, 2022 16:24
@TyriarTyriar merged commit3845008 intoxtermjs:masterJun 26, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

[8]ページ先頭

©2009-2025 Movatter.jp