Seflerz unicode #2
Conversation
- xrdp is not now built with XRDP_IBUS to allow other input methods to be more easily supported. - chansrv is only aked to start an input method if the client supports it. - chansrv sends a status report back to xrdp when asked to start and input method. - ./configure without --enable-ibus now works.
xrdp/xrdp_wm.c
Outdated
| self->client_info->unicode_input_support == UIS_ACTIVE && | ||
| self->mm->chan_trans->status == TRANS_STATUS_UP) | ||
| { | ||
| xrdp_mm_send_unicode_to_chansrv(self->mm, |
There was a problem hiding this comment.
There is no return status for "xrdp_mm_send_unicode_to_chansrv" to tell unicode processing failure here for the fall back in line 1746. Therefore it's possible that the fall back not working. For example, the unicode character is fail to process in the unicode thread but it does exist in the keymap.
My suggestion is to put the unicode procedure in the end of the function. You know, the result is the same if the character is existing in keymap, but it's more stable in the old way, I think.
There was a problem hiding this comment.
I'm fine with that. I'll change it back.
xrdp/xrdp_mm.c
Outdated
| LOG(LOG_LEVEL_INFO, "Chansrv does not support Unicode input"); | ||
| break; | ||
|
|
||
| default: |
There was a problem hiding this comment.
Better specify 2 as init failure as it does in line "chansrc:893" here. Using default is unclear. Also, I think we should do a LOG_ERROR for that case.
There was a problem hiding this comment.
I can do this if you are short on time.
|
Changes implemented as discussed. I had a thought about this conversation above:-
I'm still a bit bothered about one thing; if we receive a stream of Unicode characters and some are in the keymap and some aren't, the characters will take different routes to get to the session and so they might end up in a different order. We can leave it for now, and if it does turn out to be a problem we at least know what's happening. |
Got it. |
Hi @seflerZ
As promised, I've looked into not starting the input method if we don't need to.
I've added a couple of other small changes too. Happy to discuss if you don't like them.
I've not fully tested this, as I was having trouble getting iBus working with XFCE. I'll try to get to the bottom of that so I can do some more thorough testing later on when we're getting ready to merge.
Main changes:-
XRDP_IBUSto allow other input methods to be more easily supported../configurewithout--enable-ibusnow works.Also support for
--enable-ibusis added to the CI scripts.