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

Copy keybind (Control + C) does not update clipboard on alternative keyboard layouts #1253

Closed
Col-E opened this issue Nov 15, 2024 · 7 comments · Fixed by #1255
Closed

Copy keybind (Control + C) does not update clipboard on alternative keyboard layouts #1253

Col-E opened this issue Nov 15, 2024 · 7 comments · Fixed by #1255

Comments

@Col-E
Copy link
Contributor

Col-E commented Nov 15, 2024

On a ENG keyboard layout Control C copies text in a CodeArea as one would expect.

On other keyboard layouts such as RUS the clipboard contents are never updated.

copy-pasta-without-sauce.mp4
@Col-E
Copy link
Contributor Author

Col-E commented Nov 15, 2024

It seems that GenericStyledAreaBehavior's copy key mapping is never invoked. The WellBehaved event flow is a bit confusing to me so I haven't dug into it much.

For comparison, this is a TextInput handling Control + C (what I press physically) when I have my layout set to RUS

image

Its acknowledging that this is Control + C. Same goes for other binds like Control + A to select all, etc.

@xxDark
Copy link

xxDark commented Nov 15, 2024

I investigated this further, and the comment here either is not correct, or this does not apply to Russian keyboard layout.
KeyCharacterCombination invokes getKeyCodeForChar from JavaFX Toolkit which returns 0 for any Russian character.
I locally swapped out KeyCharacterCombination to KeyCodeCombination and everything seem to work again.

        KeyCombination SHORTCUT_A = new KeyCodeCombination( A, SHORTCUT_DOWN );
        KeyCombination SHORTCUT_C = new KeyCodeCombination( C, SHORTCUT_DOWN );
        KeyCombination SHORTCUT_V = new KeyCodeCombination( V, SHORTCUT_DOWN );
        KeyCombination SHORTCUT_X = new KeyCodeCombination( X, SHORTCUT_DOWN );
        KeyCombination SHORTCUT_Y = new KeyCodeCombination( Y, SHORTCUT_DOWN );
        KeyCombination SHORTCUT_Z = new KeyCodeCombination( Z, SHORTCUT_DOWN );
        KeyCombination SHORTCUT_SHIFT_Z = new KeyCodeCombination( Z, SHORTCUT_DOWN, SHIFT_DOWN );

@xxDark
Copy link

xxDark commented Nov 15, 2024

This seems like a leftover fix back from 2019 which was supposed to fix this bug (?), but now it causes a bug instead
Further, as shown at the screenshot by @Col-E above, the character string is empty (a single NULL byte), because the event type is KEY_PRESSED and not KEY_TYPED, thus the event does not have the character associated with it.

@Jugen
Copy link
Collaborator

Jugen commented Nov 18, 2024

Thanks @Col-E for reporting, and @xxDark for investigating.

I see that the reason for using KeyCharacterCombination was in order to support Dvorak keyboards.

Do either of you know (or can test) if changing to using KeyCodeCombination will break Dvorak support or not ?

@Jugen
Copy link
Collaborator

Jugen commented Nov 18, 2024

I've updated the RichTextFX maven 1.0.0-SNAPSHOT for testing.

@xxDark
Copy link

xxDark commented Nov 18, 2024

Do either of you know (or can test) if changing to using KeyCodeCombination will break Dvorak support or not ?

I'm not aware of whether it will break Dvorak layout. Frankly, I'm not even sure how to test this, as Windows does not provide me with an option to install that keyboard layout..

@Jugen
Copy link
Collaborator

Jugen commented Nov 18, 2024

I've subsequently had a look at the JavaDoc for KeyCodeCombination versus KeyCharacterCombination and am of the opinion that it'll not break it.

Will wait a few days for any feedback and then submit a PR for the next release in about two weeks or so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants