Skip to content

Conversation

@ShikiSuen
Copy link
Contributor

@ShikiSuen ShikiSuen commented Nov 4, 2025

This continues the accidentally-closed #4398 , replacing #4396.

closing #3925 #3814

1. On macOS, `NSTextInputClient::validAttributesForMarkedText` now automatically attaches a value
  called `_rust_winit_\(CARGO_PKG_VERSION)`.

  This allows IME devs to identify whether `winit` is used in the IMKTextInput client.
  If identified, IME devs can introduce compatibility behaviors against such IMKTextInput client.
  InputMethodKit casts `NSTextInputClient` to `IMKTextInput`, but `validAttributesForMarkedText`
  is shared between these two protocols and can all be parsed as `Array<NSString>`.

  `validAttributesForMarkedText` specifically returns an `NSArray<NSAttributedStringKey>` which
  are plain NSString*; Apple treats custom keys as legal, so slipping harmless metadata like
  `_rust_winit_\(CARGO_PKG_VERSION)` in there is the one slot where such data is piggybackable.
2. On macOS, route keyDown through `NSTextInputContext::handleEvent` before raw dispatch,
  suppress stray ASCII from keyUp, and drop stale raw characters when IME commits transformed text
  (e.g. "." -> "。"), aligning character delivery with Cocoa and third‑party IMEs.
  • Tested on all platforms changed // cargo +nightly test -p winit-appkit only.
  • Added an entry to the changelog module 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 // Only tested with cargo +nightly fmt ; cargo +nightly build ; cargo +nightly run -p winit --example ime

The true reason for the 1st modification in this PR (as listed above) is to allow IME devs to automatically enable their customized popup composition buffer window in lieu of preedit (inline composition buffer). The piggybacked metadata contains the version information of the winit package so IME devs can identify buggy winit releases in their IMEs and do their individual accomodations. If this metadata is missing or not detected, then IME can use client bundle identifiers instead (which is comparatively more difficult, still.).

@ShikiSuen
Copy link
Contributor Author

Unit test logs on my mac:

~/Repos/winit> cargo +nightly test -p winit-appkit
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.04s
     Running unittests src/lib.rs (target/debug/deps/winit_appkit-787491e69be708a2)

running 6 tests
test app::tests::test_override ... ok
test app::tests::test_custom_class ... ok
test monitor::tests::invalid_monitor_handle ... ok
test monitor::tests::from_invalid_id ... ok
test monitor::tests::monitorhandle_from_zero ... ok
test monitor::tests::uuid_stable ... ok

test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

   Doc-tests winit_appkit

running 3 tests
test winit-appkit/src/lib.rs - EventLoopBuilderExtMacOS::with_default_menu (line 515) ... ok
test winit-appkit/src/lib.rs - EventLoopBuilderExtMacOS::with_activation_policy (line 493) ... ok
test winit-appkit/src/lib.rs - (line 20) ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s


running 1 test
test winit-appkit/src/lib.rs - WindowExtMacOS::is_document_edited (line 150) ... ignored

test result: ok. 0 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.00s

all doctests ran in 0.65s; merged doctests compilation took 0.20s

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

I can not comment much whether it'll work or not, so some general comment regarding general code comprehension.

Also, have you able to verify that it actually solves the issue and doesn't affect regular Qwerty input (including hotkey handling) and that output is pretty much the same?

@ShikiSuen ShikiSuen force-pushed the shiki/fix-nstextinputclient-impl branch from 0515c33 to 9017e89 Compare November 5, 2025 03:32
@ShikiSuen
Copy link
Contributor Author

ShikiSuen commented Nov 5, 2025

@kchibisov I rebased the commit history of this PR so I can separate the out-of-topic unit test fixes to another PR.

Changes against winit-appkit/src/view.rs now have inline comments. See 16560cf.

@ShikiSuen ShikiSuen force-pushed the shiki/fix-nstextinputclient-impl branch from 9017e89 to 16560cf Compare November 5, 2025 03:33
@ShikiSuen
Copy link
Contributor Author

I found a new way to test whether ASCII inputs are handled: cargo +nightly run -p winit --example ime.

Test results showing that some extra fixes are needed. I'll push the fix later.

@ShikiSuen ShikiSuen marked this pull request as draft November 5, 2025 11:58
@ShikiSuen ShikiSuen force-pushed the shiki/fix-nstextinputclient-impl branch 3 times, most recently from 48f904e to f03165e Compare November 5, 2025 15:26
@ShikiSuen ShikiSuen marked this pull request as ready for review November 5, 2025 15:30
@ShikiSuen ShikiSuen requested a review from kchibisov November 5, 2025 15:35
@ShikiSuen
Copy link
Contributor Author

ShikiSuen commented Nov 5, 2025

This PR is defibrillated now (see e47d639 ). Everything is green. Tests in this video shows that ASCII inputs and the Sogou-style IMEs (Sogou, WeType (WeChat IME), RIME Squirrel) are all passed with no failure of committing the right punctuations / characters.

ScreenRec.2025-11-05.23.44.36.mp4

Note: On macOS we must assume that IME has nothing "disabled". This PR uses Ground state for all input methods, incl. macOS built-in ASCII inputs (ABC or EN-US).

@ShikiSuen ShikiSuen force-pushed the shiki/fix-nstextinputclient-impl branch from f03165e to e47d639 Compare November 5, 2025 15:40
@jrmoulton
Copy link

jrmoulton commented Nov 5, 2025

I haven't reviewed the code but I've tested these changes. Regular text input works but while the IME is active modifier keys do not work so as is this is still a breaking change.

(Tested using dev branch of Floem ui library here.)

@ShikiSuen
Copy link
Contributor Author

@jrmoulton Thanks for your involvement.
Please tell me your modified key combinations. I'll try troubleshooting this on Thursday.
My timezone is GMT+9 and it's my sleeping time now.

@ShikiSuen
Copy link
Contributor Author

@jrmoulton Please also test 182250d3. I just pushed that.

@jrmoulton
Copy link

It is now working perfectly for me

@ShikiSuen ShikiSuen changed the title appkit: improve the implementation of NSTextInputClient with better NSEvent handling. appkit: improve API interaction with InputMethodKit. Nov 5, 2025
- This allows IME devs to identify whether `winit` is used in the IMKTextInput client. If identified, IME devs can introduce compatibility behaviors against such IMKTextInput client. // InputMethodKit casts `NSTextInputClient` to `IMKTextInput`.
- On macOS, route keyDown through `NSTextInputContext::handleEvent` before raw dispatch, suppress stray ASCII from keyUp, and drop stale raw characters when IME commits transformed text (e.g. "." -> "。"), aligning character delivery with Cocoa and third‑party IMEs.
@ShikiSuen ShikiSuen force-pushed the shiki/fix-nstextinputclient-impl branch from 32a0983 to e5c0945 Compare November 6, 2025 08:52
@ShikiSuen
Copy link
Contributor Author

I hate "merge-commits". I rebased the PR to let them merged before my commits in this PR. Now this PR's HEAD is still the same as what @jrmoulton tested recently.

@jrmoulton
Copy link

Can confirm that these changes work for my use cases

@ShikiSuen
Copy link
Contributor Author

@kchibisov I guess my works are all done here unless you found anything I need to amend.

@kchibisov
Copy link
Member

I'd need to test in somehow(don't own macOS) before merging, or @madsmtm can probably take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants