Review of "Improved OSK" version 13

Details Page Preview

Makes Gnome's OnScreen Keyboard more usable. Features: * Includes additional buttons: Arrow keys, Esc, Tab, Ctrl, Alt, Super, F1-12 * Supports key combinations like `Ctrl + C`, `Alt + Tab`, `Ctrl + Shift + C`, `Super + A` etc. * Configurable keyboard size (landscape/portrait) * Statusbar indicator to toggle keyboard * Works in Gnome password modals * Works on Lock screen (see README for instructions) * Works on Login screen (see README for instructions) This extension is a fork of https://extensions.gnome.org/extension/3330/improved-onscreen-keyboard/ by SebastianLuebke.

Extension Homepage
https://github.com/nick-shmyrev/improved-osk-gnome-ext

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

All Versions

Version Status
18 Active
17 Active
16 Rejected
15 Active
14 Rejected
13 Rejected
12 Active
11 Inactive
10 Rejected
9 Active
8 Active
7 Active
6 Active
5 Active
4 Active
3 Rejected
2 Rejected
1 Active

Previous Reviews on this Version

JustPerfection rejected
Remove session modes or follow the ego review guidelines: https://gjs.guide/extensions/review-guidelines/review-guidelines.html#session-modes btw, `gdm` only works for system extensions not ego extensions.
NickShmyrev posted a review
Thanks for the review! After looking through the guide, a few questions/comments: "It MUST be necessary for the extension to operate correctly." Without specifying "session-modes", OSK doesn't render any buttons if screen locks and shows a gnome screensaver login dialog. Adding 'unlock-dialog' session mode fixed that issue. Would this be a good justification for keeping this option? "All signals related to keyboard events MUST be disconnected in unlock-dialog session mode." I'm not sure if I understand this part. There are a few `button.connect('commit'`, `button.connect('released'`, etc. lines in this extension, but since this is an OnScreen Keyboard extension, wouldn't disconnecting them break it? "btw, `gdm` only works for system extensions not ego extensions." The idea was to allow this extension to run on Gnome's login screen, if it was installed as a system extension. I've left some instructions on how to install is as a system extension in the README. But if this is against the rules, I can remove it and let the users add "gdm" session mode manually if they'd like to run this as a system extension.
JustPerfection posted a review
1. You can add session modes (not `gdm`) but add the reason(s) to disable function comment. 2. Line 44 shouldn't get executed for `unlock-dialog`. 3. You should disconnect those key signals in unlock dialog. and we discussed about your extension in GNOME Matrix channel, you can read it in: https://matrix.to/#/#extensions:gnome.org
NickShmyrev posted a review
Sorry, but I'm still confused about disconnecting keyboard events in unlock-dialog session mode. The whole point of this extension is to replace/extend the original Gnome OSK. It overrides methods of the original Gnome OSK, including some of the original OSK keyboard signals. If I disconnect those, the OSK simply stops working. Just to reiterate, I'm not adding any new keyboard signals, only overriding some of the existing ones. Is there any kind of workaround I could use for a situation like this? Perhaps I misunderstood the requirements, and I need to disconnect then reconnect the signals?
andyholmes waiting for author
> Sorry, but I'm still confused about disconnecting keyboard events in unlock-dialog session mode. All signals related to user input and unlocking the session have to be disconnected on the `unlock-dialog`. Allowing extensions to run on the lock screen allows them to keep operating in the session, while removing privileged items from e.g. the panel (such as removing the ability to change the Wi-Fi network on the lock screen). They are also permitted to make changes to the UI and behaviour of lock screen. Allowing third-party extensions to be involved with the actual unlocking process, however, is a very different situation. If your extension is intended to work even in the `gdm` session mode, maybe it is just best distributed in package managers, so that distributions can confirm it is secure? > The whole point of this extension is to replace/extend the original Gnome OSK. Is it crucial for this to operate on the lock screen, or will the original OSK work for the short task of unlocking the session? > Perhaps I misunderstood the requirements, and I need to disconnect then reconnect the signals? This always needs to be done. Even if your extension is permitted to run in the `user` and `unlock-dialog` modes, it will still be disabled and re-enabled when switching between session modes.
NickShmyrev posted a review
> Allowing third-party extensions to be involved with the actual unlocking process, however, is a very different situation. If your extension is intended to work even in the `gdm` session mode, maybe it is just best distributed in package managers, so that distributions can confirm it is secure? I'm fine with removing the 'gdm' session mode for EGO package. Would it be okay to leave some instructions in the README for users on how to manually enable 'gdm' session mode and install this extension system-wide if they so choose? > Is it crucial for this to operate on the lock screen, or will the original OSK work for the short task of unlocking the session? I suppose the original OSK will do for the login screen, so I'll remove 'gdm' session mode. But without enabling 'unlock-dialog' session mode, OSK breaks on Gnome's Screensaver login screen and doesn't render any buttons. That's the reason I wanted to enable the 'unlock-dialog' session mode. > Even if your extension is permitted to run in the `user` and `unlock-dialog` modes, it will still be disabled and re-enabled when switching between session modes. Thanks! The part about the extension being disabled and re-enabled when switching session modes is probably the piece I was missing. So then my question would be this: Can I still connect keyboard_event-related signals (specifically, the ones on lines 103, 109 and 146 in extension.js file) when enabling the extension, provided I disconnect these signals in disable() function?
JustPerfection waiting for author
OSK breaks on a GNOME Shell with no extensions (in screensaver login screen)?
NickShmyrev posted a review
It doesn't break with no extensions installed. But when this extension IS installed (not as a system-wide extension, just as a user extension), it works fine in user session, but breaks in screensaver login screen. OSK background panel shows up, but it doesn't have any buttons. These are the errors I can see in journalctl: JS ERROR: Exception in callback for signal: groups-changed: TypeError: this._keyboardController.getCurrentGroup is not a function _updateKeys@resource:///org/gnome/shell/ui/keyboard.js:1811:59 _onGroupChanged@resource:///org/gnome/shell/ui/keyboard.js:1816:14 _onKeyboardGroupsChanged@resource:///org/gnome/shell/ui/keyboard.js:1830:14 _emit@resource:///org/gnome/gjs/modules/core/_signals.js:114:47 _onSourcesModified@resource:///org/gnome/shell/ui/keyboard.js:2194:14 _emit@resource:///org/gnome/gjs/modules/core/_signals.js:114:47 _inputSourcesChanged@resource:///org/gnome/shell/ui/status/keyboard.js:620:14 reload@resource:///org/gnome/shell/ui/status/keyboard.js:365:14 _ibusSetContentType@resource:///org/gnome/shell/ui/status/keyboard.js:700:14 _emit@resource:///org/gnome/gjs/modules/core/_signals.js:114:47 _setContentType@resource:///org/gnome/shell/misc/ibusManager.js:277:14 JS ERROR: Exception in callback for signal: active-group: TypeError: this._keyboardController.getCurrentGroup is not a function _updateKeys@resource:///org/gnome/shell/ui/keyboard.js:1811:59 _onGroupChanged@resource:///org/gnome/shell/ui/keyboard.js:1816:14 _emit@resource:///org/gnome/gjs/modules/core/_signals.js:114:47 _onSourceChanged@resource:///org/gnome/shell/ui/keyboard.js:2200:14 _emit@resource:///org/gnome/gjs/modules/core/_signals.js:114:47 _currentInputSourceChanged@resource:///org/gnome/shell/ui/status/keyboard.js:452:14 activateInputSource@resource:///org/gnome/shell/ui/status/keyboard.js:492:14 _emit@resource:///org/gnome/gjs/modules/core/_signals.js:114:47 activate@resource:///org/gnome/shell/ui/status/keyboard.js:60:14 _inputSourcesChanged@resource:///org/gnome/shell/ui/status/keyboard.js:625:33 reload@resource:///org/gnome/shell/ui/status/keyboard.js:365:14 _ibusSetContentType@resource:///org/gnome/shell/ui/status/keyboard.js:700:14 _emit@resource:///org/gnome/gjs/modules/core/_signals.js:114:47 _setContentType@resource:///org/gnome/shell/misc/ibusManager.js:277:14 If I add 'unlock-dialog' session mode, the modified OSK shows up in screensaver login screen and works as expected. Also, a couple of questions: Would it be okay to leave some instructions in the README for users on how to manually enable 'gdm' session mode and install this extension system-wide if they so choose? Can I still connect keyboard_event-related signals (specifically, the ones on lines 103, 109 and 146 in extension.js file) when enabling the extension, provided I disconnect these signals in disable() function?
JustPerfection waiting for author
1. You can add the gdm instruction to the readme file. 2. No. any keyboard related modification is not allowed in unlock dialog. 3. That error seems like a racy situation. Maybe try to move `disable_overrides()` to the first line of the disable. or seems it doesn't get reverted back to the original. Sometimes these kind of errors caused by weird things, Maybe instead of `Keyboard.KeyboardController.prototype["getCurrentGroup"]` try `Keyboard.KeyboardController.prototype.getCurrentGroup`. btw, sorry for answering late, I'm very busy these days. you will get a faster response if you ask there in GNOME Matrix channel though: - [GNOME Matrix Channel](https://matrix.to/#/#extensions:gnome.org) - IRC Bridge: irc://irc.gimpnet.org/shell-extensions
JustPerfection rejected
Newer version approved.