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

Implement Caps Lock warning for password input #3646

Merged
merged 1 commit into from
May 3, 2020

Conversation

phoerious
Copy link
Member

@phoerious phoerious commented Oct 21, 2019

Type of change

  • ✅ New feature (non-breaking change which adds functionality)

Description and Context

This PR adds a warning to the password edit field if cAPS lOCK is enabled. Related to #3487.

As of now, the functionality works on Linux (X11), macOS, and Windows. Wayland is missing because the API sucks.

The change requires the main application to be linked against Qt5::X11Extras and libX11. This is pretty ugly, but I don't want to make this a separate .so either. That will be solved if we move OSUtils to a separate library in the future.

Resolves #2250

Screenshots

image

Testing strategy

Manual

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]
  • ✅ All new and existing tests passed. [REQUIRED]
  • ✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]

@phoerious phoerious added this to the v2.6.0 milestone Oct 21, 2019
@phoerious phoerious requested a review from droidmonkey October 21, 2019 08:54
@phoerious phoerious force-pushed the feature/capslock-warning branch 4 times, most recently from 081460a to d274916 Compare October 21, 2019 12:05
@phoerious
Copy link
Member Author

phoerious commented Oct 21, 2019

Alright, macOS works now, too and Windows has been tested as well. It's astounding how both Windows and macOS can do that in one line, but good-ol' X11 always makes you run the extra mile. Only thing missing now is a Wayland implementation.

I also changed the polling mechanism from timer-based to overriding the event() method. That way the state doesn't neatly update in near-realtime anymore (only on explicit events like show, focus, hover, keypress, etc.), but I think it's worth it, since it reduces the amount of unnecessary polling significantly.

@phoerious phoerious force-pushed the feature/capslock-warning branch 5 times, most recently from d1f7815 to 8f92c2d Compare October 21, 2019 19:03
@phoerious phoerious force-pushed the feature/capslock-warning branch from e472c95 to c293bea Compare October 31, 2019 02:36
@droidmonkey
Copy link
Member

Are you going to pick this back up?

@phoerious
Copy link
Member Author

phoerious commented Jan 5, 2020

This is basically ready, I just hate the Wayland implementation. I already depend on a third-party library (the first point I don't like at all) and it is still 40 LOC instead of one like on other sane platforms. An implementation with the plain Wayland C API would add another 100-300 LOC of boilerplate callback code.

@droidmonkey droidmonkey force-pushed the feature/capslock-warning branch from ef56738 to d7d1672 Compare January 6, 2020 02:38
@droidmonkey
Copy link
Member

droidmonkey commented Jan 28, 2020

CI is missing some dependencies:

  CMake Error at src/CMakeLists.txt:333 (find_package):
    By not providing "FindKF5Wayland.cmake" in CMAKE_MODULE_PATH this project
    has asked CMake to find a package configuration file provided by
    "KF5Wayland", but CMake did not find one.
  
    Could not find a package configuration file provided by "KF5Wayland" with
    any of the following names:
  
      KF5WaylandConfig.cmake
      kf5wayland-config.cmake
  
    Add the installation prefix of "KF5Wayland" to CMAKE_PREFIX_PATH or set
    "KF5Wayland_DIR" to a directory containing one of the above files.  If
    "KF5Wayland" provides a separate development package or SDK, be sure it has
    been installed.

Also need to run make format

@droidmonkey
Copy link
Member

droidmonkey commented Apr 7, 2020

I would like to merge this but NOT add the Wayland support for now (requires a new dependency and the code is ugly as sin). Can you extract the Wayland portion into a separate PR for 2.7.0?

@phoerious
Copy link
Member Author

phoerious commented Apr 8, 2020

Yeah, we can do that. I would want to redesign the message a little, but we may do that in another PR as well (the unlock dialogue needs some fresh air anyway). Colours also need to be adjusted to meet contrast requirements.

@phoerious phoerious force-pushed the feature/capslock-warning branch 4 times, most recently from 089fe91 to ce67c84 Compare May 2, 2020 18:52
@phoerious phoerious requested a review from droidmonkey May 2, 2020 18:53
@phoerious phoerious force-pushed the feature/capslock-warning branch from ce67c84 to f374bc9 Compare May 2, 2020 18:59
@phoerious phoerious changed the title WIP: Implement Caps Lock warning for password input Implement Caps Lock warning for password input May 2, 2020
@phoerious phoerious force-pushed the feature/capslock-warning branch from f374bc9 to 13825b7 Compare May 2, 2020 19:57
@phoerious phoerious force-pushed the feature/capslock-warning branch from 13825b7 to 8f598e5 Compare May 2, 2020 20:31
@phoerious phoerious merged commit d9214db into develop May 3, 2020
@phoerious phoerious deleted the feature/capslock-warning branch May 3, 2020 07:59
@wolframroesler
Copy link
Contributor

With those changes I'm getting a build error on Linux:

/home/wolfram/src/keepassxc/src/gui/osutils/nixutils/NixUtils.cpp:25:10: fatal error: qpa/qplatformnativeinterface.h: No such file or directory
   25 | #include <qpa/qplatformnativeinterface.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Any idea what to do against it?

@phoerious
Copy link
Member Author

Install qtbase5-private-dev.

@wolframroesler
Copy link
Contributor

Works, thanks for the quick reply :)

Should be added to https://github.com/keepassxreboot/keepassxc/wiki/Set-up-Build-Environment-on-Linux.

@wolframroesler
Copy link
Contributor

wolframroesler commented May 4, 2020

BTW, what do you think about using one of the following icons instead of exclamation-mark-in-triangle to indicate that caps-lock is on?

Screenshot from 2020-05-04 12 56 27

(apple-keyboard-caps, caps-lock, and keyboard-caps on https://materialdesignicons.com/)

EDIT: Forgot to mention that I love the new feature.

@phoerious
Copy link
Member Author

I tried the keyboard-caps icon before, but it's too thin and hard to see and also hard to interpret. The warning triangle was much clearer.

@droidmonkey
Copy link
Member

I noticed the pop-up tool tip obscures the repeat password box. This could annoy people who actually use caps lock instead of shift. They are out there...

@phoerious
Copy link
Member Author

We could place it on top, perhaps. But then it obscures the label.

@droidmonkey
Copy link
Member

It's fine

droidmonkey added a commit that referenced this pull request Jul 7, 2020
Added

- Custom Light and Dark themes [#4110, #4769, #4791, #4796, #4892, #4915]
- Compact mode to use classic Group and Entry line height [#4910]
- View menu to quickly switch themes, compact mode, and toggle UI elements [#4910]
- Search for groups and scope search to matched groups [#4705]
- Save Database Backup feature [#4550]
- Sort entries by "natural order" and move lines up/down [#4357]
- Option to launch KeePassXC on system startup/login [#4675]
- Caps Lock warning on password input fields [#3646]
- Add "Size" column to entry view [#4588]
- Browser-like tab experience using Ctrl+[Num] (Alt+[Num] on Linux) [#4063, #4305]
- Password Generator: Define additional characters to choose from [#3876]
- Reports: Database password health check (offline) [#3993]
- Reports: HIBP online service to check for breached passwords [#4438]
- Auto-Type: DateTime placeholders [#4409]
- Browser: Show group name in results sent to browser extension [#4111]
- Browser: Ability to define a custom browser location (macOS and Linux only) [#4148]
- Browser: Ability to change root group UUID and inline edit connection ID [#4315, #4591]
- CLI: `db-info` command [#4231]
- CLI: Use wl-clipboard if xclip is not available (Linux) [#4323]
- CLI: Incorporate xclip into snap builds [#4697]
- SSH Agent: Key file path env substitution, SSH_AUTH_SOCK override, and connection test [#3769, #3801, #4545]
- SSH Agent: Context menu actions to add/remove keys [#4290]

Changed

- Complete replacement of default database icons [#4699]
- Complete replacement of application icons [#4066, #4161, #4203, #4411]
- Complete rewrite of documentation and manpages using Asciidoctor [#4937]
- Complete refactor of config files; separate between local and roaming [#4665]
- Complete refactor of browser integration and proxy code [#4680]
- Complete refactor of hardware key integration (YubiKey and OnlyKey) [#4584, #4843]
- Significantly improve performance when saving and opening databases [#4309, #4833]
- Remove read-only detection for database files [#4508]
- Overhaul of password fields and password generator [#4367]
- Replace instances of "Master Key" with "Database Credentials" [#4929]
- Change settings checkboxes to positive phrasing for consistency [#4715]
- Improve UX of using entry actions (focus fix) [#3893]
- Set expiration time to Now when enabling entry expiration [#4406]
- Always show "New Entry" in context menu [#4617]
- Issue warning before adding large attachments [#4651]
- Improve importing OPVault [#4630]
- Improve AutoOpen capability [#3901, #4752]
- Check for updates every 7 days even while still running [#4752]
- Improve Windows installer UI/UX [#4675]
- Improve config file handling of portable distribution [#4131, #4752]
- macOS: Hide dock icon when application is hidden to tray [#4782]
- Browser: Use unlock dialog to improve UX of opening a locked database [#3698]
- Browser: Improve database and entry settings experience [#4392, #4591]
- Browser: Improve confirm access dialog [#2143, #4660]
- KeeShare: Improve monitoring file changes of shares [#4720]
- CLI: Rename `create` command to `db-create` [#4231]
- CLI: Cleanup `db-create` options (`--set-key-file` and `--set-password`) [#4313]
- CLI: Use stderr for help text and password prompts [#4086, #4623]
- FdoSecrets: Display existing secret service process [#4128]

Fixed

- Fix changing focus around the main window using tab key [#4641]
- Fix search field clearing while still using the application [#4368]
- Improve search help widget displaying on macOS and Linux [#4236]
- Return keyboard focus after editing an entry [#4287]
- Reset database path after failed "Save As" [#4526]
- Use SHA256 Digest for Windows code signing [#4129]
- Improve handling of ccache when building [#4104, #4335]
- macOS: Properly re-hide application window after browser integration and Auto-Type usage [#4909]
- Auto-Type: Fix crash when performing on new entry [#4132]
- Browser: Send legacy HTTP settings to recycle bin [#4589]
- Browser: Fix merging browser keys [#4685]
- CLI: Fix encoding when exporting database [#3921]
- SSH Agent: Improve reliability and underlying code [#3833, #4256, #4549, #4595]
- FdoSecrets: Fix crash when editing settings before service is enabled [#4332]
@phoerious phoerious added pr: new feature Pull request that adds a new feature and removed new feature labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: new feature Pull request that adds a new feature ux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Caps Lock
3 participants