Opened 7 years ago

Last modified 3 years ago

#10017 accepted Bug Report - General

'Edit Keys' should not allow you to set keybindings that myth will ignore

Reported by: dantheperson@… Owned by: sphery
Priority: minor Milestone: unknown
Component: MythTV - General Version: 0.24-fixes
Severity: medium Keywords: keybinding Shift
Cc: Ticket locked: no

Description

The 'edit keys' part of the front-end should not allow you to set a key binding that myth is going to ignore by design, i.e. any keybinding involving SHIFT.

Even better would be if there was a config option within 'Edit Keys' to say 'do not ignore the shift key'.

As it is, 'Edit Keys' does not ignore the shift key, and so it allows you to bind Ctrl+Shift+P to a function. The rest of myth then ignores the shift key, so you are left endlessly trouble shooting your setup wondering what you have done wrong until you stumble across closed bug reports saying it is broken by design.

Either ignore the shift key consistently (i.e. ignore it within 'Edit Keys') and so pressing Ctrl+Shift+P would show and be saved as Ctrl+P, and thus it will still work when a users presses their keyboards 'multimedia' button that sends Ctrl-Shift-P, Or at least save people hours of troubleshooting by popping up a message saying that 'due to legacy case-insensitive lirc support, you cannot use the Ctrl key in keybindings, and will need to install lirc with the keyboard input driver and fiddle around with text config files to use your keyboard'

Anyways, don't mean to sound too negative, hope the suggestions here a constructive to the Journey to a better user experience. Keep up the good work!

Change History (9)

comment:1 Changed 6 years ago by beirdo

Milestone: unknown

comment:2 Changed 6 years ago by sphery

Owner: set to sphery
Status: newaccepted

comment:3 Changed 6 years ago by sphery

Priority: majorminor

resetting defaults

comment:4 Changed 4 years ago by Ken Truesdale <kentruesdale@…>

I thought it was just me until I recently did some deeper testing. I discovered that when I entered a keybinding such as a "+" key, the key that showed on the screen was "Shift++" meaning it knew I was holding down the shift key and that I generated a "+" and it put them together. To me that was always redundant terminology - on my small keyboard, I can't get a "+" without the shift key, so yeah, shift key, whatever. And I was always frustrated that keys like this didn't work. But I now realize the redundancy of the "shift" is the crux of the problem. Because I modified the database directly to change the keybinding from "Shift++" to simply "+" and it works fine. Therefore, I think the issue is in the front end, when it captures the keybinding, it seems to be working a little too hard to figure out what the user did - it should simply accept the key that it got.

comment:5 in reply to:  4 Changed 4 years ago by paulh

Replying to Ken Truesdale <kentruesdale@…>:

I thought it was just me until I recently did some deeper testing. I discovered that when I entered a keybinding such as a "+" key, the key that showed on the screen was "Shift++" meaning it knew I was holding down the shift key and that I generated a "+" and it put them together. To me that was always redundant terminology - on my small keyboard, I can't get a "+" without the shift key, so yeah, shift key, whatever. And I was always frustrated that keys like this didn't work. But I now realize the redundancy of the "shift" is the crux of the problem. Because I modified the database directly to change the keybinding from "Shift++" to simply "+" and it works fine. Therefore, I think the issue is in the front end, when it captures the keybinding, it seems to be working a little too hard to figure out what the user did - it should simply accept the key that it got.

Myth has always ignored the shift key so that 'd' and 'D' would be the same key binding. IIRC the thinking was if you allowed key and shift+key to allow different bindings users would get annoyed if they didn't realise the caps lock was on and accidentally activated the wrong binding.

It's true Myth should ignore or warn about invalid key bindings like these but on the grand scheme of things it's a pretty trivial problem which is probably why this ticket hasn't been touched in 3 years :(

As always developer resources are very scares at the moment so any help fixing the problem would be much appreciated :)

comment:6 Changed 4 years ago by Ken Truesdale <kentruesdale@…>

While it may be true that the shift key is ignored when taking action, clearly it isn't ignored when being captured and it isn't that Myth should ignore or warn about invalid key bindings (although, sure a warning about a truly invalid binding would be fine). It's that the way it is "seeing" the binding during capture that is the real problem. That was the reason for my comment - because I thought the original ticket may not have been clear on that.

Paul, thanks for the quick response. When I saw how old the ticket was, I had the same conclusion as you: 3 years with no change on a trivial matter (and one that I have now proved can be worked around through SQL) is clearly the sign of not enough resources to whack all the moles. And frankly, I probably would have chosen the same priorities - recent changes finally have .27 stable enough that the WAF is now high enough we don't use the cable box any more and that's way more important than not being able to use a "+" without using SQL. And while I'd love to take a look at the code to try to squash the bug, I'd need to start with setting up a development environment and given the time I just spent doing a rebuild of my backend, there's no time in my schedule for that. So ultimately, I personally made the same decision as others: this is low priority and I'd rather keep working on the important stuff that keeps the rest of it humming. And I figured that if I posted that adjustment to the ticket, it might help somebody who was in a "while I'm already fixing xyz, I can fix that" mode to more quickly get to the fix. Thanks again, Paul.

comment:7 Changed 4 years ago by dantheperson@…

Oh cool, thanks for your findings. So if i do a direct update to the SQL tables to remove the SHIFT key from the mapping in the DB it will match the keypress that the front sees after it filters away the SHIFT key. Then when i press the multimedia buttons on my keyboard (which sends things like Ctrl+Shift+P) it should 'just work'. Cool

comment:8 Changed 4 years ago by sphery

Yes, the proper solution is almost definitely to just remove the shift from the key grabber at https://code.mythtv.org/cgit/mythtv/tree/mythtv/programs/mythfrontend/keygrabber.cpp#n99 (and line 100). The only reason I didn't do so 3 years ago is because I haven't had time to do proper (and complete) tests.

comment:9 Changed 3 years ago by Karl Newman <SiliconFiend@…>

I made the changes from comment 8 and it seems to work. I did not test extensively though. I'm curious about the alternate solution, which would be actually recognizing the Shift key. I don't know how Qt works but does it actually show a key sent with caps lock on as having the Shift modifier active? Or an upper case character generated from lirc? Is the Shift modifier the only way Qt can distinguish upper case characters?

Note: See TracTickets for help on using tickets.