Re: RFR: 8301900: TextArea: Committing text with ENTER in an IME window inserts newline [v3]
On Fri, 2 Feb 2024 18:43:19 GMT, Martin Fox wrote: >> In the Mac glass code the presence of "marked" text (which is tracked in the >> nsAttrBuffer) signals that an IME is active. In this state the current code >> assumes that when NSTextInputContext handles a `keyDown:` it will either >> generate a call to `insertText:replacementRange:` or one of the routines >> that manipulates the marked (composed) text. But this bug shows that >> sometimes the IME acts on the event without generating any calls back into >> glass at all. >> >> In this PR the logic is simplified: if the NSTextInputContext handles the >> `keyDown:` and there's marked (composed) text we don't generate a KeyEvent. >> Otherwise we do. This PR removes the `shouldProcessKeyEvent` flag since it >> no longer assumes we can catch callbacks to update it correctly. >> >> The existing code also assumes that the composition phase ends when the >> NSTextInputContext calls `insertText:replacementRange` to commit the text. >> This is true but if the user tries to use a dead-key sequence to generate a >> non-existent character (like an accented 'q') the context will call >> `insertText` twice while handling one key down event. In that case we can't >> exit the composition mode upon seeing the first `insertText` call since it >> will cause us to mis-handle the second one. This PR defers exiting >> composition mode until the end of `keyDown:`. >> >> I also updated a few deprecated constants so this file no longer generates >> compiler warnings. > > Martin Fox has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains seven commits: > > - Now handling ESC ending the composition state. Minor setEnabled cleanup. > - Merge remote-tracking branch 'upstream/master' into macimefixes > - When IM enabled state changes we dismiss the IM window. > - Merge remote-tracking branch 'upstream/master' into macimefixes > - Comment tweaks > - Merge remote-tracking branch 'upstream/master' into macimefixes > - Modified method of determing which keyDowns are sent as KeyEvents. Marked as reviewed by kcr (Lead). - PR Review: https://git.openjdk.org/jfx/pull/1351#pullrequestreview-1898820773
Re: RFR: 8301900: TextArea: Committing text with ENTER in an IME window inserts newline [v3]
On Fri, 2 Feb 2024 18:43:19 GMT, Martin Fox wrote: >> In the Mac glass code the presence of "marked" text (which is tracked in the >> nsAttrBuffer) signals that an IME is active. In this state the current code >> assumes that when NSTextInputContext handles a `keyDown:` it will either >> generate a call to `insertText:replacementRange:` or one of the routines >> that manipulates the marked (composed) text. But this bug shows that >> sometimes the IME acts on the event without generating any calls back into >> glass at all. >> >> In this PR the logic is simplified: if the NSTextInputContext handles the >> `keyDown:` and there's marked (composed) text we don't generate a KeyEvent. >> Otherwise we do. This PR removes the `shouldProcessKeyEvent` flag since it >> no longer assumes we can catch callbacks to update it correctly. >> >> The existing code also assumes that the composition phase ends when the >> NSTextInputContext calls `insertText:replacementRange` to commit the text. >> This is true but if the user tries to use a dead-key sequence to generate a >> non-existent character (like an accented 'q') the context will call >> `insertText` twice while handling one key down event. In that case we can't >> exit the composition mode upon seeing the first `insertText` call since it >> will cause us to mis-handle the second one. This PR defers exiting >> composition mode until the end of `keyDown:`. >> >> I also updated a few deprecated constants so this file no longer generates >> compiler warnings. > > Martin Fox has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains seven commits: > > - Now handling ESC ending the composition state. Minor setEnabled cleanup. > - Merge remote-tracking branch 'upstream/master' into macimefixes > - When IM enabled state changes we dismiss the IM window. > - Merge remote-tracking branch 'upstream/master' into macimefixes > - Comment tweaks > - Merge remote-tracking branch 'upstream/master' into macimefixes > - Modified method of determing which keyDowns are sent as KeyEvents. As far as I can tell, the new behavior matches the native app one. 有難う - Marked as reviewed by angorya (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/1351#pullrequestreview-1864065082
Re: RFR: 8301900: TextArea: Committing text with ENTER in an IME window inserts newline [v3]
On Fri, 2 Feb 2024 18:43:19 GMT, Martin Fox wrote: >> In the Mac glass code the presence of "marked" text (which is tracked in the >> nsAttrBuffer) signals that an IME is active. In this state the current code >> assumes that when NSTextInputContext handles a `keyDown:` it will either >> generate a call to `insertText:replacementRange:` or one of the routines >> that manipulates the marked (composed) text. But this bug shows that >> sometimes the IME acts on the event without generating any calls back into >> glass at all. >> >> In this PR the logic is simplified: if the NSTextInputContext handles the >> `keyDown:` and there's marked (composed) text we don't generate a KeyEvent. >> Otherwise we do. This PR removes the `shouldProcessKeyEvent` flag since it >> no longer assumes we can catch callbacks to update it correctly. >> >> The existing code also assumes that the composition phase ends when the >> NSTextInputContext calls `insertText:replacementRange` to commit the text. >> This is true but if the user tries to use a dead-key sequence to generate a >> non-existent character (like an accented 'q') the context will call >> `insertText` twice while handling one key down event. In that case we can't >> exit the composition mode upon seeing the first `insertText` call since it >> will cause us to mis-handle the second one. This PR defers exiting >> composition mode until the end of `keyDown:`. >> >> I also updated a few deprecated constants so this file no longer generates >> compiler warnings. > > Martin Fox has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains seven commits: > > - Now handling ESC ending the composition state. Minor setEnabled cleanup. > - Merge remote-tracking branch 'upstream/master' into macimefixes > - When IM enabled state changes we dismiss the IM window. > - Merge remote-tracking branch 'upstream/master' into macimefixes > - Comment tweaks > - Merge remote-tracking branch 'upstream/master' into macimefixes > - Modified method of determing which keyDowns are sent as KeyEvents. Two additions to the code. I was not handling the case where the keystroke didn't commit text but did clear the marked text (getting us out of composition mode). You can do this by repeatedly pressing ESC. When the enabled state changes we shouldn't have any marked text but if we do it should just be discarded. The old code was trying to deal with focusOwner changes but that cleanup has to happen before the enabled state changes instead of after (which is why finishInputMethodComposition exists). - PR Comment: https://git.openjdk.org/jfx/pull/1351#issuecomment-1924483704
Re: RFR: 8301900: TextArea: Committing text with ENTER in an IME window inserts newline [v3]
> In the Mac glass code the presence of "marked" text (which is tracked in the > nsAttrBuffer) signals that an IME is active. In this state the current code > assumes that when NSTextInputContext handles a `keyDown:` it will either > generate a call to `insertText:replacementRange:` or one of the routines that > manipulates the marked (composed) text. But this bug shows that sometimes the > IME acts on the event without generating any calls back into glass at all. > > In this PR the logic is simplified: if the NSTextInputContext handles the > `keyDown:` and there's marked (composed) text we don't generate a KeyEvent. > Otherwise we do. This PR removes the `shouldProcessKeyEvent` flag since it no > longer assumes we can catch callbacks to update it correctly. > > The existing code also assumes that the composition phase ends when the > NSTextInputContext calls `insertText:replacementRange` to commit the text. > This is true but if the user tries to use a dead-key sequence to generate a > non-existent character (like an accented 'q') the context will call > `insertText` twice while handling one key down event. In that case we can't > exit the composition mode upon seeing the first `insertText` call since it > will cause us to mis-handle the second one. This PR defers exiting > composition mode until the end of `keyDown:`. > > I also updated a few deprecated constants so this file no longer generates > compiler warnings. Martin Fox has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits: - Now handling ESC ending the composition state. Minor setEnabled cleanup. - Merge remote-tracking branch 'upstream/master' into macimefixes - When IM enabled state changes we dismiss the IM window. - Merge remote-tracking branch 'upstream/master' into macimefixes - Comment tweaks - Merge remote-tracking branch 'upstream/master' into macimefixes - Modified method of determing which keyDowns are sent as KeyEvents. - Changes: https://git.openjdk.org/jfx/pull/1351/files Webrev: https://webrevs.openjdk.org/?repo=jfx=1351=02 Stats: 60 lines in 3 files changed: 35 ins; 10 del; 15 mod Patch: https://git.openjdk.org/jfx/pull/1351.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1351/head:pull/1351 PR: https://git.openjdk.org/jfx/pull/1351
