Skip to content

on MacOS, Fix not sending ReceivedCharacter event for some key combination#1347

Merged
vbogaevsky merged 4 commits intorust-windowing:masterfrom
hatoo:macos-fix-ReceivedCharacter
Jan 14, 2020
Merged

on MacOS, Fix not sending ReceivedCharacter event for some key combination#1347
vbogaevsky merged 4 commits intorust-windowing:masterfrom
hatoo:macos-fix-ReceivedCharacter

Conversation

@hatoo
Copy link
Copy Markdown
Contributor

@hatoo hatoo commented Dec 31, 2019 •
edited
Loading

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

This PR fixes #1267
It seems that Ctrl+Q is some prefix key when hasMarkedText returns NO.
For example, it emits '\u{1e}' when input Ctrl+Q and Arrow-up key.
So I modified hasMarkedText to returns YES always.
I'm not 100% sure about the mean of this change, but I think it's OK because hasMarkedText returns always YES after inputting Option+N in previous implementation.

A key with Command no longer emits ReceivedCharacter event by this change.

@hatoo hatoo changed the title Macos fix received character on MacOS, Fix not sending ReceivedCharacter event for some key combination Dec 31, 2019
@goddessfreya goddessfreya added DS - appkit Affects the AppKit/macOS backend C - waiting on maintainer A maintainer must review this code labels Jan 2, 2020
Osspial
Osspial reviewed Jan 4, 2020
View reviewed changes
@Osspial Osspial force-pushed the macos-fix-ReceivedCharacter branch from 22a8f6f to da3cfcf Compare January 5, 2020 19:21
vbogaevsky
vbogaevsky reviewed Jan 13, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@vbogaevsky vbogaevsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On PR branch combinations of cmd, option & ctrl with any symbol keys return KeyboardInput events.
@chrisduerr @jayache80, does this PR resolve your issue?

@chrisduerr
Copy link
Copy Markdown
Contributor

chrisduerr commented Jan 13, 2020

I was not able to reproduce, but @jeremydr2 was able to share an event log with the missing events. Maybe he can create another log by running cargo run --example window in this repo for comparison to see if it got fixed?

@ghost
Copy link
Copy Markdown

ghost commented Jan 14, 2020 •
edited by ghost
Loading

I hit C-q once. Here are the logs:

NewEvents(WaitCancelled { start: Instant { t: 202776246335713 }, requested_resume: None })
2020-01-14 13:32:48,664 TRACE [winit::platform_impl::platform::view] Triggered `flagsChanged`
2020-01-14 13:32:48,664 TRACE [winit::platform_impl::platform::view] Completed `flagsChanged`
WindowEvent { window_id: WindowId(Id(140631488875984)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 62, state: Pressed, virtual_keycode: Some(RControl), modifiers: CTRL }, is_synthetic: false } }
DeviceEvent { device_id: DeviceId(DeviceId), event: ModifiersChanged(CTRL) }
MainEventsCleared
RedrawRequested(WindowId(Id(140631488875984)))
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { t: 202778853666746 }, requested_resume: None })
2020-01-14 13:32:48,706 TRACE [winit::platform_impl::platform::view] Triggered `keyDown`
2020-01-14 13:32:48,707 TRACE [winit::platform_impl::platform::view] Triggered `hasMarkedText`
2020-01-14 13:32:48,707 TRACE [winit::platform_impl::platform::view] Completed `hasMarkedText`
2020-01-14 13:32:48,707 TRACE [winit::platform_impl::platform::view] Triggered `hasMarkedText`
2020-01-14 13:32:48,707 TRACE [winit::platform_impl::platform::view] Completed `hasMarkedText`
2020-01-14 13:32:48,707 TRACE [winit::platform_impl::platform::view] Completed `keyDown`
WindowEvent { window_id: WindowId(Id(140631488875984)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 12, state: Pressed, virtual_keycode: Some(Q), modifiers: CTRL }, is_synthetic: false } }
MainEventsCleared
RedrawRequested(WindowId(Id(140631488875984)))
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { t: 202778896226323 }, requested_resume: None })
2020-01-14 13:32:48,832 TRACE [winit::platform_impl::platform::view] Triggered `keyUp`
2020-01-14 13:32:48,832 TRACE [winit::platform_impl::platform::view] Completed `keyUp`
WindowEvent { window_id: WindowId(Id(140631488875984)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 12, state: Released, virtual_keycode: Some(Q), modifiers: CTRL }, is_synthetic: false } }
MainEventsCleared
RedrawRequested(WindowId(Id(140631488875984)))
RedrawEventsCleared
NewEvents(WaitCancelled { start: Instant { t: 202779021806898 }, requested_resume: None })
2020-01-14 13:32:48,855 TRACE [winit::platform_impl::platform::view] Triggered `flagsChanged`
2020-01-14 13:32:48,855 TRACE [winit::platform_impl::platform::view] Completed `flagsChanged`
WindowEvent { window_id: WindowId(Id(140631488875984)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 62, state: Released, virtual_keycode: Some(RControl), modifiers: (empty) }, is_synthetic: false } }
DeviceEvent { device_id: DeviceId(DeviceId), event: ModifiersChanged((empty)) }
MainEventsCleared
RedrawRequested(WindowId(Id(140631488875984)))

@vbogaevsky , Looks to me like it works on Mac!
Let me know if there are other key combinations you would like me to test.
My issue was only related to C-q, but I'm happy to be a typing monkey

@chrisduerr
Copy link
Copy Markdown
Contributor

chrisduerr commented Jan 14, 2020

Thanks a lot for testing, looks like this should be good to go then!

@vbogaevsky vbogaevsky merged commit 9daa073 into rust-windowing:master Jan 14, 2020
kchibisov added a commit to kchibisov/winit that referenced this pull request Mar 7, 2020
@kchibisov
Revert "on MacOS, Fix not sending ReceivedCharacter event for some ke…
8b47e6e 
…y combination (rust-windowing#1347)"

This reverts commit 9daa073.

This commit introduced other bug rust-windowing#1453 with likely much more common bindings,
so reverting it for now.

Fixes rust-windowing#1453.
@iceiix iceiix mentioned this pull request Dec 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@Osspial Osspial Osspial left review comments

@vbogaevsky vbogaevsky vbogaevsky approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

C - waiting on maintainer A maintainer must review this code DS - appkit Affects the AppKit/macOS backend

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MacOS not sending ReceivedCharacter event

5 participants

@hatoo @chrisduerr @vbogaevsky @Osspial @goddessfreya