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

Shell app: plus key not functional on Finnish keyboard layout #1214

Closed
ghost opened this issue Jun 11, 2021 · 10 comments · Fixed by #1363
Closed

Shell app: plus key not functional on Finnish keyboard layout #1214

ghost opened this issue Jun 11, 2021 · 10 comments · Fixed by #1363
Labels
bug Existing functionality not working as expected component/shell

Comments

@ghost
Copy link

ghost commented Jun 11, 2021

When using Firefox ESR with a Finnish keyboard layout, the plus key (+) does not output anything, with modifiers or without. From investigating this, it seems that having a different keyboard layout changes some of the keycodes. Changing the "var keycapEP = 61;" on line 6952 of hterm_all_1.85.mod.js into "var keycapEP = 171;" will make the key work. Removing the following lines will also fix this issue:

      this.addKeyDefs(
        // Firefox Italian +*.
        [171, '+*', DEFAULT, c('onPlusMinusZero_'), DEFAULT, c('onPlusMinusZero_')]
      ); 

After changing the keycapEP value, some keys print weird errors in the javascript console when using the US layout (even though all keys seem to work) so it is not a proper fix. As for removing the Italian layout kludge, I am not sure.

There are newer versions of hterm available as well, at the moment we cannot really test if this has been fixed there though. As most of our users are using the Finnish layout, this is a bit of a blocker at the moment.

@oscwiag oscwiag added this to the Needs Triaged milestone Jun 11, 2021
@johrstrom johrstrom added component/shell bug Existing functionality not working as expected labels Jun 11, 2021
@johrstrom
Copy link
Contributor

Thanks for the ticket. Looks like we'd have to fix it for several OS/browser variants. You mention FF here on what OS?

A quick search of the hterm bugs list didn't show much.

@johrstrom johrstrom modified the milestones: Needs Triaged, Backlog Jun 11, 2021
@ghost
Copy link
Author

ghost commented Jun 14, 2021

We have so far tested this on following FF versions and platforms:

Linux: FF 78.10.0esr and 78.11.0esr
Windows: FF 89.0 and nightly
macOS: FF 88.0.1 and 89.0

None of these are working at the moment so this should be pretty widely reproducible.

Steps to reproduce:
Change to a Finnish keyboard layout, start the shell app
Type '+' on the keyboard (the key right next to the zero key)

What we expect would happen:
You'd get a '+' sign in the terminal (or a '?' with shift etc)

What happens:
Nothing.

Keycode issues are probably not unique to just the Finnish keyboard layout but for obvious reasons, that's the one we are testing.

For comparison, we tried this on chromium (90.0.4430.212) and there the plus sign works but a few other keys including the Scandinavian characters (åäö) do not. However, we may have to open a separate bug about those as hterm seems to use at least somewhat different code on FF than it does on others. Or should we expand this bug to cover keyboard layout issues on other browsers too?

For reference here's the Finnish keyboard map: https://en.wikipedia.org/wiki/QWERTY#/media/File:KB_Sweden.svg (yeah, it says Swedish but they're identical).

@ayushchatur
Copy link
Contributor

some interesting observation, the "+" key on the numpad works just fine with the mentioned firefox on linux/windows,
this issue is however reproducible on versions later than mentioned also

@CSC-swesters
Copy link
Contributor

Thanks @ayushchatur for letting us know that you are able to reproduce this!

Indeed, the numpad + works as intended. However, this is not a viable workaround. Primarily, because the ? and \ characters which are written with Shift+"+" and AltGr+"+" respectively, are also unavailable due to this bug. Secondarily, because laptop users don't necessarily have a numpad.

@ayushchatur
Copy link
Contributor

Well i digged a bit in it, the key mapping mentioned only for firefox
this.addKeyDefs( // Firefox Italian +*. [171, '+*', DEFAULT, c('onPlusMinusZero_'), DEFAULT, c('onPlusMinusZero_')] );
doesnt seem to get this defined action called upon,
for that to happen it need to be registered of all browsers. So i tried to move it out of the if condition on the firefox and this seem to work perfectly because there is no conflict with 171 code any where
@johrstrom what do you think about #1357

@ayushchatur ayushchatur mentioned this issue Aug 23, 2021
@johrstrom
Copy link
Contributor

Can you checkout #1363?

I'm actually having trouble replicating. I've tried all sorts of flavours of Finnish dialects in Windows language settings and it all seems to successfully print + and ? keys. It does however fail to print the key to the right of the one in question (back and forward ticks).

I did however find something else a bit troubling which you allude to in the stack traces when this fails. As an example Chrome seems to hit that stack trace and fail on emojis, so there's something a bit more wrong here than just this.

@johrstrom johrstrom mentioned this issue Aug 24, 2021
@johrstrom
Copy link
Contributor

Yea there's definitely a larger bug here with all sorts of non en-US characters like vowels with umlauts.

We'll definitely fix this issue with + then defer the larger issue with all these other characters.

@CSC-swesters
Copy link
Contributor

We quickly retested #1363 on our environment on Linux and Windows. It seems to be working just as well as #1358.

Regarding Jeff's trouble with the back and forward ticks, this is work perfectly on both Windows and Linux with Firefox on our side. On Chromium-based browsers, however, they don't work. Note that a single backtick is written by pressing the key next to backspace, and then pressing the spacebar. Or alternatively backtick + a letter. In itself, the backtick key doesn't output a letter, it needs another key to output something. See Dead key on Wikipedia for a better explanation 😄

For example, all of the following characters work in the Shell app on Firefox, but not on Chromium (verified all of these on Linux):

åäö        <-- å, ä, ö keys on Finnish/Swedish keyboards
áéíóúý     <-- forward tick + letter
àèìòùỳ     <-- back tick + letter
`          <-- back tick + space
äëïöüÿ     <-- umlaut + letter
âêîôûŷ     <-- circumflex + letter
ãẽĩõũỹ     <-- tilde + letter
ȩç         <-- cedille + letter
¸          <-- cedille + space

To clarify, the original issue with the plus key was only happening on Firefox. With your patches, this issue is fixed. Once this gets merged, Firefox seems to work fine, while Chromium-based browsers have a lot of problems (as stated above).

@johrstrom
Copy link
Contributor

Thank you for the confirmation. I'm going to cut a release today and get this pushed out to you soon.

That said - as we've indicated - there are lots of keys that seem to be broken which I'll create a new ticket for. If any other key that's critical for your use like + please let us know. I'll create a ticket shortly for the much larger 'many non US keys are broken' bug.

@johrstrom
Copy link
Contributor

v2.0.16 is available on the latest repo. I'm not sure if it'll make it to the general public in the 2.0 repo as it has #1369 - the superset of this issue. I've got to dig deeper into that issue to see if we can't fix as it the shell app may be broken for many Chrome users.

Micket added a commit to c3se/ondemand that referenced this issue Sep 30, 2021
johrstrom pushed a commit that referenced this issue Oct 1, 2021
* Update hterm to version 1.91

* Update hterm fix for undefined this.screen to 1.91

See OSC/ood-shell#64

* Reapply patch for +-key issue #1214 on Finnish keyboard layouts

See #1214

* Add hterm bugfix to shell's README

* Move hterm keycode 171 fix below second row.
johrstrom pushed a commit that referenced this issue Mar 3, 2023
* Update hterm to version 1.91

* Update hterm fix for undefined this.screen to 1.91

See OSC/ood-shell#64

* Reapply patch for +-key issue #1214 on Finnish keyboard layouts

See #1214

* Add hterm bugfix to shell's README

* Move hterm keycode 171 fix below second row.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Existing functionality not working as expected component/shell
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants