Input handler gets triggered endlessly on macOS embedding

[dnfield]

The problem is here: https://github.com/TerminalStudio/xterm.dart/blob/774abaf75413ad93c2325a66705fd1428ef53d38/lib/frontend/input_listener.dart#L183-L187

Handling this kind of thing is unfortunately a bit more complicated. The C++ code ends up thinking it needs to keep updating you, and it sends an update which causes you to also send an update, and it goes back and forth in a loop. The expectation is that if you send an update and get a new message, you won't send an update again if the values have not changed. You can see how the framework handles this here: https://github.com/flutter/flutter/blob/f0c9710493d3431a1983967dc26fad2a8513b94e/packages/flutter/lib/src/widgets/editable_text.dart#L1958-L1966

I'm surprised this isn't causing bigger problems on other platforms, but maybe it is. At any rate, the problem on macos is very noticable if you add a print to the onInput in the example - it will print indefinitely as soon as interaction starts with the application.

[dnfield]

An input handler, of course, could ignore empty strings - but you'll find your application doing tons of extra work to serialize and send those messages back and forth in a loop like that.

[devmil]

Are you on the beta channel? I have seen this behavior also but it went away when switching back to the stable channel. There is a "fix" for this in the Composing state PR (https://github.com/TerminalStudio/xterm.dart/pull/17) that just returns null whenever the text is empty to interrupt the endless loop.

[devmil]

Btw: You might be the perfect person to explain the best approach to handle the composing/preedit state of multi-key characters like â, right? :)

[dnfield]

I'm on a relatively recent commit to master. If this is a regression Flutter introduced we may want to file a bug there :\ It does seem strange that it happens only on macOS.

haha I'm not quite sure about being the right person for that. I assume if you're getting them as separate keystrokes, you could just maintain a buffer and combine them when you're supposed to - but I'm not quite sure how that works on various desktop systems.

[devmil]

I'm on a relatively recent commit to master. If this is a regression Flutter introduced we may want to file a bug there :\ It does seem strange that it happens only on macOS.

Ok. Might be worth a deeper investigation to find the exact conditions this happens.

haha I'm not quite sure about being the right person for that. I assume if you're getting them as separate keystrokes, you could just maintain a buffer and combine them when you're supposed to - but I'm not quite sure how that works on various desktop systems.

Too bad :) I was hoping for some guidance as the solution I have in the already linked PR doesn't feel like the intended solution to that problem. I had to do similar strange things in my Qt based Terminal application but that doesn't rule out the possibility that I simply don't have grasped the core concept yet 🤣

[dnfield]

This was introduced by https://github.com/flutter/engine/pull/24533 - @cbracken was this intentional breakage? It didn't seem to break the framework use case.

[dnfield]

@devmil - I've only experienced this as a user, not a developer, but you get two key inputs. @cbracken would probably know better about how this is handled because I'm sure he's done similar work for it with other langauges - though I'm not entirely sure if those work much differently or not :)

[cbracken]

There was no intentional change to this behaviour in the engine, but the code to close the loop and report back the current state to the framework has always been there -- this is the relevant before/after of my change: https://github.com/flutter/engine/pull/24533/files#diff-bf62e3c20fb34cec64858961d271a62ccbde5a2bd822f7e5747d7b5886b63ff2L108-L113

We could (and probably should) do something a little smarter here like only fire an update back to the framework if the update to the engine caused a text editing state change -- i.e. some form of dirty-tracking. I'm a bit confused as to what might have changed in my patch that would trigger a call to updateEditState where we hadn't been triggering it before. :/

[cbracken]

Didn't update here but the difference appears to be due to the fact that the common text input model doesn't model invalid ranges such as -1,-1. There are two ranges we care about in the model:

• Composing range: used to model the range in which IME composing is occuring (e.g. inputting Japanese text prior to converting the text to kanji). This range only exists during composing, so we model a boolean state and return an invalid -1,-1 range to the framework when we're not composing.
• Selection range: used to model the selection (e.g. 3,5) or, where there is no selection, the cursor position (e.g. 3,3). There are no cases that I can think of where we have an active text input connection, but no selection/cursor position, so the text input model doesn't model such a scenario.

TextEditingValue objects model their selection/composing ranges as -1,-1 by default for new objects, but in the embedders, we typically model them the way the native platform does -- on some platforms this means clamping them to the actual text range -- examples: Linux, iOS, Windows.

[dnfield]

So do we just need to update the defaults for TextEditingValue objects?