Fix problems with the hide/restore functionality on the tray icon (issue #1595).#1700
Fix problems with the hide/restore functionality on the tray icon (issue #1595).#1700joanbm wants to merge 22 commits intokeepassxreboot:release/2.3.2from joanbm:develop
Conversation
Correct compile time issues with 2.3.0
* Fix build on NetBSD.
Building with -flto caught the fact that we were ignoring the return value of readMagicNumbers(), which potentially left the value of 'sig2' uninitialized. Signed-off-by: Steven Noonan <steven@uplinklabs.net>
KdbxReader::readDatabase: abort if reading magic numbers fails
* Tests: Speed up AutoType testing Decrease default autotype delay to 1 to improve test suite speed by seconds. This shaves multiple seconds off the whole test suite. In some cases, the largest part. Also, initialize config just creating the test instance, just in case that it ever depends on the configuration values at that point already. * Tests: Speed up Kdbx4 testing This speeds up the Kdbx4 tests by using parameters optimized for speed for the key derivation functions. On an i7-6700K the tests run close to 50% faster with this change (about 1.5s vs. 3s).
|
This seems ok, can someone test on MacOS? @weslly maybe |
|
Please rebase onto |
src/main.cpp
Outdated
| bool minimizeOnStartup = config()->get("GUI/MinimizeOnStartup").toBool(); | ||
| bool minimizeToTray = config()->get("GUI/MinimizeToTray").toBool(); | ||
| if (minimizeOnStartup) { | ||
| if (minimizeOnStartup && !minimizeToTray) { |
There was a problem hiding this comment.
If this particular issue only affects Linux, the extra condition shouldn't be enforced on other OSes.
Also, the "minimize on startup" checkbox should be disabled when "minimize to tray" is active at the settings window on Linux.
There was a problem hiding this comment.
Why? I want KeePassXC to start minimized in the tray.
There was a problem hiding this comment.
@weslly It shouldn't hurt if the condition is enforced in other platforms, though I will add an #ifdef for it if it makes the reason clearer. Also a comment referencing it, now that I think of it.
I concur with @phoerious, "minimize on startup" + "minimize to tray" should be a valid combination, it's specially useful if you have set KeePassXC to auto unlock on boot (and want it to be on the tray upon boot).
There was a problem hiding this comment.
@phoerious Whoops, I just noticed this extra condition doesn't matter since the window is only shown on the next if
|
@droidmonkey You're right, I missed the git flow branch structure and did the PR over develop, will fix. |
|
You need to put your changes into their own branch. It would be easiest for you to make a new branch off 2.3.2 and cherry pick your commits into that new branch. Push it, then fix which branches this PR is pointing to. |
|
@droidmonkey Unfortunately I can't change the source branch of the PR, only the destination branch... not sure if there is any solution. Sorry for the annoyance, I will make sure to do it right the next time. |

Fix/work around tray issues on some Linux systems such as Arch Linux, and Windows. Fixes all cases explained in issue #1595.
Description
Basically there are two separate issues at play there:
This can happen both on start (this is the case of the original bug report), if
minimizeOnStartup=trueandminimizeToTray=true, then the window is minimized but not shown:Or when clicking on the tray icon to hide the window, the window is both minimized and hidden by code on
hideWindow()(this is what is causing my flicker-restore-minimize cycle sometimes):toggleWindow(), there's aQApplication::activeWindow() == thischeck:On Windows, when clicking on the tray icon when the window is visible,
QApplication::activeWindow() = NULL. I think this is due to differences in window focus behavior between Windows and Linux (I believe Windows removes the focus from the window when clicking on the tray).Motivation and context
This change is required in order to avoid issues when hiding and restoring the KeePassXC windows from tray. One of those issues happen on Windows, and the other in some Linux systems (what exactly causes some systems and desktop environments to be affected and some not to be affected is not known, though it probably depends on the Qt version, but must also depend on some environment condition since on my system the bug only happens sometimes).
For the Linux issue, I avoid both minimizing and hiding the window at the same time by code. It should suffice to only hide the window if the event is caused by a tray icon click.
For the Windows issue, I just removed the check for QApplication::activeWindow() == this, I think it is unreliable and the other checks in the same line (which were added later) should suffice alone.
Fixes issue #1595 (#1595).
How has this been tested?
I set up some VMs for manual testing of the issue, and I found this:
Environment details:
I also ran all the automatic tests on my local Arch Linux system and they passed.
Screenshots (if appropriate):
Windows issue: https://imgur.com/fJW62v7
Linux issue (flicker on first restore): https://imgur.com/cES8olj
Types of changes
Checklist:
-DWITH_ASAN=ON. [REQUIRED]