smithay
smithay copied to clipboard
Allow forcibly setting text-input activation state
This patch adds a new InputMethodHandle::set_active method, which allows setting the input method to either force-on, force-off, or automatic (based on text-input).
Opening a draft because there's three things I'd like some feedback on:
Firstly I've only tested this with virtual keyboard. I'd assume it should work just as well for IME but I'm not aware of its intricacies so feedback would be welcome on this.
Secondly this currently requires a clone on the client side to be used, but I'm not sure there's a better API possible:
let cseat = self.seat.clone();
cseat.input_method().set_active(self, None, self.ime_override);
If the seat isn't stored on the state the clone wouldn't be necessary obviously, but I'd assume this is usually where it is stored.
Thirdly currently force-disabling IME works just fine, but force-enabling IME doesn't work. When force-enabling IME it won't go away once it is opened, but it will not open my virtual keyboard if nothing is focused. I thought for a second this might be because no surface is passed to activate_input_method, but I don't see why it would require a surface considering that zwp_input_method_v1::activate doesn't require any surfaces. This is the only thing I consider critical enough to keep this in the draft stage.
Codecov Report
Attention: 62 lines in your changes are missing coverage. Please review.
Comparison is base (
3af1e3c) 20.87% compared to head (02f73ed) 21.36%.
| Files | Patch % | Lines |
|---|---|---|
| src/wayland/input_method/input_method_handle.rs | 0.00% | 60 Missing :warning: |
| src/wayland/text_input/text_input_handle.rs | 0.00% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #1313 +/- ##
==========================================
+ Coverage 20.87% 21.36% +0.49%
==========================================
Files 155 155
Lines 25264 25297 +33
==========================================
+ Hits 5273 5405 +132
+ Misses 19991 19892 -99
| Flag | Coverage Δ | |
|---|---|---|
| wlcs-buffer | 18.48% <0.00%> (-0.03%) |
:arrow_down: |
| wlcs-core | 18.16% <0.00%> (?) |
|
| wlcs-output | 7.69% <0.00%> (?) |
|
| wlcs-pointer-input | 20.22% <0.00%> (+0.02%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thirdly currently force-disabling IME works just fine, but force-enabling IME doesn't work. When force-enabling IME it won't go away once it is opened, but it will not open my virtual keyboard if nothing is focused. I thought for a second this might be because no surface is passed to activate_input_method, but I don't see why it would require a surface considering that zwp_input_method_v1::activate doesn't require any surfaces. This is the only thing I consider critical enough to keep this in the draft stage.
The way that all these IME stuff works is that activate/deactivate requires done, however once you do force enable, you're likely not sending the done. In normal auto flow, it's naturally dispatched from the commit. Keep also in mind that it's a bit tricky, since your synthetic done shouldn't advance any serial I think.
In general, you can always see the WAYLAND_DEBUG log from the IME itself, but I bet that done is kind of missing.
Firstly I've only tested this with virtual keyboard. I'd assume it should work just as well for IME but I'm not aware of its intricacies so feedback would be welcome on this.
It'll certainly break the normal input operation, I think, since you have clients not supporting text-input and IME also tends to take grab, thus what you propose is very tricky to get right, though I haven't tested your patch yet with real IME.
In general, if you keep IME around, but your client won't be capable of IME input(no text-input) smithay won't understand that, I think, since you disable the logic for activate/deactivate from the focus handler in seat/keyboard.rs.
The way that all these IME stuff works is that activate/deactivate requires done, however once you do force enable, you're likely not sending the done. In normal auto flow, it's naturally dispatched from the commit. Keep also in mind that it's a bit tricky, since your synthetic done shouldn't advance any serial I think.
There is no zwp_input_method_v1::done event. That's exclusive to text-input and is not used by squeekboard at all. So I don't see how done would be required here?
In general, if you keep IME around, but your client won't be capable of IME input(no text-input) smithay won't understand that, I think, since you disable the logic for activate/deactivate from the focus handler in seat/keyboard.rs.
Yeah that's mainly what I'm concerned about, since this deals only with input-method and not with text-input.
There is no zwp_input_method_v1::done event. That's exclusive to text-input and is not used by squeekboard at all. So I don't see how done would be required here?
I retract that statement. It doesn't show up in the wayland explorer but it clearly is a real event:
// Works
[1084773.565] [email protected]()
[1084773.683] [email protected]()
// Doesn't
[1087873.565] [email protected]()
I'd assume this is likely due to Wayland explorer only documenting zwp_input_method_v1.
Well, the issue with clients not using text-input is still kind of present. The main issue is that IME can grab input, and if client doesn't support IME input it won't ever have input in the end, because the input will be grabbed.
If IME does virtual-keyboard protocol, it may work, however when it grabs and only passes via text-input means it certainly won't.
So if you want such setting to work, you should only allow force enable when client can do text-input in the first place, and once it can't you mask the thing, otherwise you'll stuck without any input at all. Maybe you don't really care about such case and you want users to manually figure that out with UI/bindings in their compositor, but it's not really nice.
I have abandoned this idea in favor of allowing the user to bind to one of three virtual keysyms with an action to manually set the virtual keyboard to enabled/disabled/auto.
Since most virtual keyboards have a dedicated side-channel for manual control, this should provide a good solution. It also allows force-enabling keyboards at which point they can switch to the virtual keyboard protocol themselves instead of trying to use IME when it is not possible.
https://github.com/catacombing/catacomb/commit/cb74f8dddc1bd71fdb322a0f948d46101ae225d0