On Wed, 13 Sep 2023 15:52:43 GMT, Martin Fox <d...@openjdk.org> wrote:
>> Thiago Milczarek Sayao has updated the pull request with a new target base >> due to a merge or a rebase. The pull request now contains 74 commits: >> >> - Rework to use process_key >> - Merge branch 'master' into new_ime >> >> # Conflicts: >> # modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp >> - Rework to use process_key >> - Don't highlight for dead keys >> - Don't move the caret on preedit >> - Add function to return relative location of the caret (it's how it's >> handled on Linux). >> - Merge branch 'master' into new_ime >> - Merge branch 'master' into new_ime >> - Weird API >> - JavaFX does not currently supports surrouding >> - ... and 64 more: https://git.openjdk.org/jfx/compare/5e145cc0...7bbfa914 > > modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp line > 91: > >> 89: void WindowContextBase::commitIME(gchar *str) { >> 90: if (!im_ctx.on_preedit) { >> 91: jchar key = g_utf8_get_char(str); > > This block is trying to set up calls to View.notifyKey. To get the correct > KeyCode requires information in the original GdkEventKey like the hardware > code and modifier state (for handling Num Lock on the key pad). The simplest > solution is to record a pointer to the original GdkEventKey before calling > `gtk_im_context_filter_keypress` and here pass that pointer to `process_key`. > It will get the KeyCode right and generate the appropriate View.notifyKey > calls. > > (In theory another approach is to ignore the commit and return 'false' from > filterIME so the original GdkEvent isn't filtered. I tried that but it turned > into a mess because I was seeing two key press events for every keystroke. > `gtk_im_context_filter_keypress` filters out the original key press event > without generating a commit. Instead it posts a duplicate event with a > private modifier flag set and the commit signal is sent out when that second > event is passed to `gtk_im_context_filter_keypress`.) I did change it, but probably need some more work. I also discovered that I probably should use the `PANGO_ATTR_*` to set the `IME_ATTR_*` with boundaries. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1080#discussion_r1328025313