Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Termux ignores Ctrl-Space on Chromebook #1333

Closed
snogglethorpe opened this issue Nov 2, 2019 · 26 comments
Closed

Termux ignores Ctrl-Space on Chromebook #1333

snogglethorpe opened this issue Nov 2, 2019 · 26 comments

Comments

@snogglethorpe
Copy link

snogglethorpe commented Nov 2, 2019

Problem description
On a Chromebook (with a hardware keyboard), typing Ctrl-Space does nothing.

This particular key-combination is traditionally bound to the set-mark command in Emacs, and so is very heavily used by most users. set-mark is also available via Ctrl-2 (and of course may be bound by the user to something else), but I think for many users, this is inconvenient, (1) because Ctrl-2 is a bit harder to type, (2) because the documentation says to use "C-SPC", and (3) most importantly, because many users have strongly developed muscle memory telling them to hit Ctrl-Space for this command.

This may be related to the fact that in most context on ChromeOS, Ctrl-Space toggles the current input method. However, this is not useful for Termux, as ChromeOS input methods are ignored (whether this is a bug or not is another question).

Other apps in ChromeOS (e.g. the ssh app) successfully use Ctrl-Space for their own use (ssh just passes it through the the connection), so it's definitely possible to override ChromeOS's default behavior for this key-combination (of course it's possible that Android apps in ChromeOS have unique constraints).

Steps to reproduce
Open Termux, run some program where Ctrl-Space should have an effect (e.g., Emacs), type Ctrl-Space. Nothing happens.

Expected behavior
Ctrl-Space should result in some sort of key sequence on the terminal. I think traditionally, Ctrl-Space yielded the same result as Ctrl-2 / Ctrl-Shift-@, but basically anything would be OK as long as it's possible for the user program to see the event.

Additional information
Updatable packages:
apt/stable 1.4.9-19 i686 [upgradable from: 1.4.9-15]
bash/stable 5.0.11 i686 [upgradable from: 5.0.9]
binutils/stable 2.33.1 i686 [upgradable from: 2.32-5]
ca-certificates/stable 20191016 all [upgradable from: 20190515]
clang/stable 9.0.0 i686 [upgradable from: 8.0.1]
command-not-found/stable 1.42 i686 [upgradable from: 1.39]
coreutils/stable 8.31-8 i686 [upgradable from: 8.31-7]
curl/stable 7.66.0-1 i686 [upgradable from: 7.65.3-6]
dos2unix/stable 7.4.1 i686 [upgradable from: 7.4.0-1]
emacs/stable 26.3-1 i686 [upgradable from: 26.3]
findutils/stable 4.7.0 i686 [upgradable from: 4.6.0-4]
gdb/stable 8.3.1 i686 [upgradable from: 8.3-4]
glib/stable 2.60.7 i686 [upgradable from: 2.60.6-1]
gnutls/stable 3.6.10 i686 [upgradable from: 3.6.9-1]
libandroid-support/stable 25-2 i686 [upgradable from: 25-1]
libcurl/stable 7.66.0-1 i686 [upgradable from: 7.65.3-6]
libexpat/stable 2.2.9 i686 [upgradable from: 2.2.7-1]
libffi/stable 3.2.1-5 i686 [upgradable from: 3.2.1-4]
libgcrypt/stable 1.8.5 i686 [upgradable from: 1.8.4-1]
libgnutls/stable 3.6.10 i686 [upgradable from: 3.6.9-1]
libllvm/stable 9.0.0 i686 [upgradable from: 8.0.1]
m4/stable 1.4.18-3 i686 [upgradable from: 1.4.18-1]
ncurses/stable 6.1.20190928 i686 [upgradable from: 6.1.20190511-7]
ndk-sysroot/stable 20-6 i686 [upgradable from: 20-1]
openssh/stable 8.1p1 i686 [upgradable from: 8.0p1-4]
openssl/stable 1.1.1d-1 i686 [upgradable from: 1.1.1c-2]
strace/stable 5.3 i686 [upgradable from: 5.2]
termux-tools/stable 0.72 all [upgradable from: 0.69-2]
texinfo/stable 6.7 i686 [upgradable from: 6.6-1]
Subscribed repositories:
https://dl.bintray.com/grimler/science-packages-24 science/stable
https://dl.bintray.com/grimler/game-packages-24 games/stable
https://dl.bintray.com/termux/termux-packages-24 stable/main
System information:
Linux localhost 3.18.0-19747-gd0dcdfafc1b5 #1 SMP PREEMPT Mon Oct 21 01:01:50 PDT 2019 i686 Android
Termux-packages arch:
i686
Android version:
9
Device manufacturer:
Google
Device model:
Samsung Chromebook Pro

@khinsen
Copy link

khinsen commented Jan 8, 2020

I have the same problem running Termux plus Emacs on an Onyx Boox Note2, which is running Android 9. Tested with both a Bluetooth keyboard and a USB keyboard.

@aadcg
Copy link

aadcg commented May 30, 2020

I have the same problem. Though I can run C-SPC if I use the on-screen CTRL.

I could swear this used to work before with my external keyboard.

EDIT: I think C-SPC doesn't work because that's the key android uses to switch keyboard layouts. I have no idea how to change this. I'm running LineageOS.

@snogglethorpe
Copy link
Author

EDIT: I think C-SPC doesn't work because that's the key android uses to switch keyboard layouts.

There are other ChromeOS apps, like the ssh app, which allow use of C-SPC (in ssh's case, it just passes it through the terminal connection), so it's not an inherent ChromeOS problem.

Termux does not seem to properly handle non-English input anyway (it works.... strangely), so not having the input-method-switching binding of C-SPC available in Termux windows might not be such a big deal....

@aadcg
Copy link

aadcg commented Jun 9, 2020

so not having the input-method-switching binding of C-SPC available in Termux windows might not be such a big deal....

Yes, input-method-switching in Termux is useless. And, as you have noted, C-SPC is a big deal for Emacs users.

I suggested that if Android didn't intercept that keybinding to change the input method of the connected physical keyboard, maybe it would work. But, anyway, that wouldn't explain why it doesn't work on ChromeOS.

Just for the sake of completeness, here's a gif of the issue at hand.

ScreenRecord-2020-06-09-23-20-15

Btw, I noticed that if you run emacs -nw inside of the GNOME terminal and you hit C-h k C-SPC it says:

C-@ runs the command set-mark-command.

So, C-SPC is actually interpreted as C-@... I'm not an expert, but I believe this connected with fact that it GNOME terminal and Termux are based on the VT100 terminal.

@5bodnar
Copy link

5bodnar commented Jun 10, 2020

Like khinsen, I am also experiencing this problem on an Onyx Boox Note 2, running Android 9 and a Bluetooth keyboard (Logitech 920-008334 if it helps any). It would be wonderful to have this fixed.

@aadcg
Copy link

aadcg commented Jun 10, 2020

My inclination is still that Android (or ChromeOS) somehow intercepts C-SPC. Notice that if you use the virtual on-screen termux keys to send C-SPC it works... That makes me think that Termux' terminal emulator implementation is correct.

@bwachter
Copy link

bwachter commented Jun 23, 2020

I was looking into that for my Cosmo earlier. I noticed that ctrl-space is defined as 'fallback' in most key mappings, so I went searching what this means. The Android key character map documentation mentions "fallback : Perform a default action if the key is not handled by the application."

I didn't go looking at the termux code, but I can see ctrl-space in some keymapping apps, so I assume it should be possible for termux to handle ctrl-space to avoid triggering that fallback action.

edit went digging a bit, in KeyHandler.java there's handling of CTRL for the SPACE key. Maybe throwing in some debugging there to see if that gets triggered may be useful, though I currently don't have a spare device to easily test my own builds.

Also somebody who understands the key translation should probably look at that, shouldn't this also return modifiers instead of "\0"?

return ((keyMode & KEYMOD_CTRL) == 0) ? null : "\0";

@5bodnar
Copy link

5bodnar commented Jul 4, 2020

Thanks for digging that up. If someone can tell me what should be returned if CTRL is down when the space key is pressed I'll have a go at some debugging.

@trygveaa
Copy link
Contributor

trygveaa commented Jul 4, 2020

Also somebody who understands the key translation should probably look at that, shouldn't this also return modifiers instead of "\0"?

Not sure what you mean by return modifiers? This function returns the control sequence for a key combination, with modifiers translated. Ctrl-space is translated to \0, which is correct. But if you mean that it should include the sequence for alt if that is pressed, then yes, I see that that's missing.

Thanks for digging that up. If someone can tell me what should be returned if CTRL is down when the space key is pressed I'll have a go at some debugging.

As mentioned by others, that's already defined in KeyHandler.java and is already returning the correct sequence (apart from when alt also is pressed, as mentioned above). So you have to look into if Ctrl-space is reaching Termux at all, or how Termux can detect it.

@5bodnar
Copy link

5bodnar commented Jul 6, 2020

I did some debugging and can confirm that CNTRL + SPACE is reaching Termux. I think I've traced the problem to TerminalView.java's onKeyPreIme() method. It seems that the CNTRL + SPACE combination doesn't get processed by TerminalView's onKeyDown() method, and is instead handled by call to a super class at the end of the method, i.e.

super.onKeyPreIme(keyCode, event);

To get the desired behaviour, I appended a new else if branch, as follows:

if (LOG_KEY_EVENTS) // line 528
            Log.i(EmulatorDebug.LOG_TAG, "onKeyPreIme(keyCode=" + keyCode + ", event=" + event + ")");
if (keyCode == KeyEvent.KEYCODE_BACK) {
     ...
}
// new branch here
else if (keyCode == KeyEvent.KEYCODE_SPACE && event.isCtrlPressed()){
     return onKeyDown(keyCode, event);
}

I've tested it on emacs and it appears to work. Could someone comment on if this is the right way to go? Thanks.

@5bodnar
Copy link

5bodnar commented Jul 7, 2020

Would the normal procedure be to commit my changes locally and generate a pull request? Sorry, I'm new to contributing to github.

@trygveaa
Copy link
Contributor

trygveaa commented Jul 7, 2020

Yes, click the button in github to create a fork, create a branch in that fork, commit the changes, push them and open a PR.

@Grimler91
Copy link
Member

@5bodnar I can confirm that your patch makes ctrl + space work on devices that it was previously broken! However, it breaks ctrl+space on devices where it was previously working.

Ctrl+space has always worked on my tablet, but with this patch it seem to send ctrl+space twice, most of the time. I can see "Mark set" in emacs, instantly followed by "Mark deactivated", when I press ctrl+space in emacs.
On my phone however the patch makes ctrl+space work most of the time (but not always).

5bodnar added a commit to 5bodnar/termux-app that referenced this issue Jul 11, 2020
@5bodnar
Copy link

5bodnar commented Jul 11, 2020

@trygveaa Thanks for the quick instructions. I've cloned, branched, and committed, but I guess before a PR I'd better resolve @Grimler91's issue.

@Grimler91 Thanks for testing and nice to hear it's a fix for devices that were previously broken. Bummer that it breaks previously working devices of course. My first thought is that maybe it is due to different Android versions? Would you be able to share the version of Android running on your tablet (where the patch breaks Ctrl+space) ? I'd also be grateful for any other thoughts on what the problem might be / how to procede.

@bwachter
Copy link

Can you try what happens if you just return true? Your patch lets other listeners handle the key event as well, so my guess would be that on the systems with two keypresses registered this is not registered as a fallback key / no IME switch is available, which then makes Android pass in the event a second time. On the systems where it didn't work before that second event is eaten up by the IME switcher.

@5bodnar
Copy link

5bodnar commented Jul 13, 2020

@bwachter Thanks, I'll give that a try and report back.

@Grimler91
Copy link
Member

I have opened a PR to add a crude workaround for this, with @5bodnar's patch, that I intend to merge within a few days unless there are objections.

After it has been merged it will be possible to write ctrl-space-workaround=true in ~/.termux/termux.properties and then run termux-reload-settings (or restart termux) to make ctrl+space work.

@Grimler91
Copy link
Member

I mentioned in my previous comment

On my phone however the patch makes ctrl+space work most of the time (but not always).

I think this has to do with how fast the buttons are pressed, pressing ctrl + space relatively slowly always works, but when fast termux seem to fail to recognize the key combination.

@bwachter
Copy link

bwachter commented Jan 2, 2021

@Grimler91 I assume you're making it configurable because of your issue with double key presses? If so, check my comment about the event being passed on to other listeners - I don't have an environment I can easily check myself, but I believe if passing on is prevented it should also not break devices where it currently works, which might make the setting obsolete.

@Grimler91
Copy link
Member

Grimler91 commented Jan 2, 2021

@bwachter

I assume you're making it configurable because of your issue with double key presses?

No, mostly to not break ctrl + space on devices where it works without the workaround

Do you with

if you just return true

mean

diff --git a/terminal-view/src/main/java/com/termux/view/TerminalView.java b/terminal-view/src/main/java/com/termux/view/TerminalView.java
index b9df06f..32f453e 100644
--- a/terminal-view/src/main/java/com/termux/view/TerminalView.java
+++ b/terminal-view/src/main/java/com/termux/view/TerminalView.java
@@ -567,6 +567,8 @@ public final class TerminalView extends View {
                         return onKeyUp(keyCode, event);
                 }
             }
+        } else if (keyCode == KeyEvent.KEYCODE_SPACE && event.isCtrlPressed()) {
+            return true;
         }
         return super.onKeyPreIme(keyCode, event);
     }

?

If so, I tried it now and ctrl + space does not work unfortunately

@bwachter
Copy link

bwachter commented Jan 2, 2021

if you just return true

mean

diff --git a/terminal-view/src/main/java/com/termux/view/TerminalView.java b/terminal-view/src/main/java/com/termux/view/TerminalView.java
index b9df06f..32f453e 100644
--- a/terminal-view/src/main/java/com/termux/view/TerminalView.java
+++ b/terminal-view/src/main/java/com/termux/view/TerminalView.java
@@ -567,6 +567,8 @@ public final class TerminalView extends View {
                         return onKeyUp(keyCode, event);
                 }
             }
+        } else if (keyCode == KeyEvent.KEYCODE_SPACE && event.isCtrlPressed()) {
+            return true;
         }
         return super.onKeyPreIme(keyCode, event);
     }

?

If so, I tried it now and ctrl + space does not work unfortunately

I think so, but it's been half a year now since I've read this part of the Android documentation to comment here. IIRC you were expected to return true to signal you're handling the event yourself, or the key event to pass it onto other input event handlers if you're not handling it yourself or it is something where it's not harmful if others also react to it.

@Grimler91
Copy link
Member

v0.105 should be available from fdroid within hours or days, (as long as the fdroid CI build succeeds). After updating to that version you can set ctrl-space-workaround = true in ~/.termux/termux.properties and then run termux-reload-settings to be able to use ctrl + space.

@aadcg
Copy link

aadcg commented Jan 9, 2021

Is this fix working for everybody? Setting ctrl-space-workaround to true has no effect for me.

@ghost
Copy link

ghost commented Jan 9, 2021

@aadcg Fix is in version 0.105 which is not available on F-Droid currently.

@aadcg
Copy link

aadcg commented Jan 25, 2021

C-SPC now runs the set-mark-command in Emacs, woohoo!

However, this key event cycles through input methods. I'm on LineageOS,
is anyone facing this issue and found a workaround? Thanks.

@khinsen
Copy link

khinsen commented Apr 10, 2021

Works fine on my Onyx Boox Note2, thanks!

@ghost ghost locked and limited conversation to collaborators Oct 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants