Disable keyboard switching with a shortcut in boot.iso #6090
Conversation
jkonecny12
temporarily deployed
to
gh-cockpituous
GitHub Actions
Inactive
|
Hello @jkonecny12! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2025-01-21 12:48:43 UTC |
jkonecny12
force-pushed
the
main-block-compositor-keyboard-shortcut
branch
from
256665b to
9437cef
Compare
jkonecny12
temporarily deployed
to
gh-cockpituous
GitHub Actions
Inactive
|
/kickstart-test --testtype smoke |
|
Would the disabling of keyboard switching persist beyond installation? If so, that'd be bad. (I don't know offhand if this is one of the cases where the config used by anaconda also gets 'persisted' into the installed system, or not). The other note I have is that I don't think the check is strong enough, we have some lives that are still on X.org where presumably this still works? Not that I've actually tried it. |
I will test it to make sure but it shouldn't. That is one of the main reasons why I split the code. This should touched only part solving compositor (current environment). |
|
/kickstart-test --testtype smoke |
|
It seems to be working correctly, it is just interesting to me that Gnome Shell doesn't look to react and still showing the |
|
uff, if it says 'en' even when we select a different keyboard, that's not great :/ The test we have assumes that switching actually works, it wouldn't intelligently notice that switching was disabled and adapt (openQA isn't great at that kind of thing). But I could test it manually I guess. |
So my main question still remains. Do we want to get this merged as temporary solution? I guess the |
|
sorry, I had not had chance to look at it yet. in theory I am OK with this as a bodge. In practice the "it doesn't tell you the right current keyboard if it's not english" thing sounds a bit worrying. |
don't get me wrong, it works just fine in Anaconda. It doesn't in the installed system if you switch layouts with keyboard shortcut but that seems unrelated. With this patch Anaconda works as expected it just disable switching layouts by keyboard shortcut, you have to do that by clicking on an icon but the icon works correctly. |
|
/build-image --boot.iso |
|
@AdamWill I'm building you an ISO here in the PR so you can easily test it to understand what am I talking about. |
|
Images built based on commit 9437cef:
Download the images from the bottom of the job status page. |
|
thanks. I know how to build ISOs, I just had rather a lot else going on :D if it behaves as intended, I think that's good. |
pyanaconda/ui/gui/spokes/keyboard.py
Outdated
| @@ -502,6 +503,10 @@ def _refresh_switching_info(self): | |||
| "supported when using RDP.\n" | |||
| "However the settings will be used " | |||
| "after the installation.")) | |||
| elif not conf.system.supports_compositor_keyboard_layout_shortcut: | |||
| self._layoutSwitchLabel.set_text(_("Keyboard layouts switching by " | |||
| "keyboard shortcuts is not supported " | |||
There was a problem hiding this comment.
I'd say is "not currently supported", but I don't really know, this would need en eye of someone with good english so feel free to ignore...
There was a problem hiding this comment.
Good point. I'll ask Sagar for grammar review.
|
|
||
| @property | ||
| def supports_compositor_keyboard_layout_shortcut(self): | ||
| """Does the compositor supports keyboard layout options correctly?""" |
There was a problem hiding this comment.
I'll change the comment but I'm leaving it in the method name as it is used in the method above the same way.
jkonecny12
force-pushed
the
main-block-compositor-keyboard-shortcut
branch
from
9437cef to
6352cff
Compare
jkonecny12
temporarily deployed
to
gh-cockpituous
GitHub Actions
Inactive
Let's use inheritance to show which code is used for installation and what for compositor control. It increases the readability and improve modifications for the future use. To make it correct it is split into Base class and two specific classes. That way it is crystal clear what is used for what and there should not be constant fighting between different usages and side effects. As a side effect this change will fix a potential issue when installed system which doesn't selected `options` during the installation will use options from the running system. This is harder to achieve state but it could happen.
jkonecny12
force-pushed
the
main-block-compositor-keyboard-shortcut
branch
from
6352cff to
e5cde67
Compare
jkonecny12
temporarily deployed
to
gh-cockpituous
GitHub Actions
Inactive
|
/kickstart-test --testtype smoke |
|
Adding a screenshot of the changed UI.
|
This is a workaround for https://issues.redhat.com/browse/RHEL-71880. We need this workaround to avoid users confusion. Without this the keyboard switching will work but it is not visible to users, so they could easily switch layouts without noticing it and wrote passwords wrong.
jkonecny12
force-pushed
the
main-block-compositor-keyboard-shortcut
branch
from
e5cde67 to
be32044
Compare
jkonecny12
temporarily deployed
to
gh-cockpituous
GitHub Actions
Inactive
|
Improved string after Sagar suggestion. Thanks! |
|
/kickstart-test --testtype smoke |
|
/kickstart-test --testtype keyboard |
|
Looks great, thank you. |
The keyboard switching recently migrated to localed. However, there is a bug reported that the keyboard switching with a keyboard shortcut doesn't work. However, that is not completely true, the keyboard switching correctly works but is not propagated to localed so Anaconda is not aware of the layout switch.
This behavior is much more problematic. Even reporters of the bug assumed that the keyboard switching doesn't work at all. Users could easily drop into a trap of using different keyboard layout on passwords during the installation then what they think is used.
Manual keyboard switching from Anaconda still works as expected. This is workaround which should be removed when bugs on gnome-kiosk are resolved:
NOTE:
Part of this change is code re-factorization which splits the
LocaledWrapperclass based on the usage (compositor / installation). This change improves readability but also fixes potential bug when xkboptions from compositor are stored to the installed system instead of the user defined ones.