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

Main window pops up after browser selection dialog #2579

Closed
jo-so opened this issue Jan 5, 2019 · 15 comments · Fixed by #2583
Closed

Main window pops up after browser selection dialog #2579

jo-so opened this issue Jan 5, 2019 · 15 comments · Fixed by #2583

Comments

@jo-so
Copy link

jo-so commented Jan 5, 2019

Since »Switch browser integration to use native raising of windows« (5488f1b) the main window pops up after the dialogue for selecting the credentials.

  1. Start KeePassXC and unlock the database
  2. Put KeePassXC into tray-icon mode (hide the window)
  3. Go to a website that asks for a password.
  4. After closing the dialogue the main window pops up.
@varjolintu
Copy link
Member

Fixed it. You can give the PR a try if you like to test it.

@jo-so
Copy link
Author

jo-so commented Jan 6, 2019

I'm sorry, but PR #2583 doesn't fix my problem. After applying the patch of the PR the main window still pops up after the dialogue.

@varjolintu
Copy link
Member

Just for confirmation, do you hide the window using Cmd+H or is it minimized to the dock? Can you do a screen recording of this? I cannot reproduce it.

@jo-so
Copy link
Author

jo-so commented Jan 6, 2019 via email

@varjolintu
Copy link
Member

I still cannot reproduce it. The only time the main window will pop up is if the application is activated or the browser window is hidden. This triggers the previous window to activate (macOS behaviour).

@varjolintu
Copy link
Member

So this is happens in a Linux enviroment? What is your window manager? I thought we were talking about macOS. You removed the issue template, so please write it again.

@jo-so
Copy link
Author

jo-so commented Jan 9, 2019 via email

@jo-so
Copy link
Author

jo-so commented Jan 21, 2019

I've tried the develop branch b59fd6d and the bug described in this issue isn't fixed.

@varjolintu
Copy link
Member

varjolintu commented Jan 21, 2019

This has something to do with AwesomeWM. I tried the same case with i3 and Cinnamon and I couldn't reproduce the problem. I'm pretty certain AwesomeWM is not actually hiding the window but doing something else.

EDIT: But I'll recheck it of course because it's something that worked previously.

@jo-so
Copy link
Author

jo-so commented Feb 9, 2019

I've debugged into this problem. The raise of the window gets triggered by getMainWindow()->showMinimized() in BrowserService::hideWindow().

(gdb) bt 4
#0  BrowserService::hideWindow (this=0x563b36dc79d0) at /home/joerg/git/keepassxc/src/browser/BrowserService.cpp:1012
#1  0x0000563b36831a58 in BrowserService::confirmEntries (this=0x563b36dc79d0, pwEntriesToConfirm=..., url=..., host=..., submitHost=..., realm=...) at /home/joerg/git/keepassxc/src/browser/BrowserService.cpp:717
#2  0x0000563b3682eff3 in BrowserService::findMatchingEntries (this=0x563b36dc79d0, id=..., url=..., submitUrl=..., realm=..., keyList=..., httpAuth=false) at /home/joerg/git/keepassxc/src/browser/BrowserService.cpp:324
#3  0x0000563b36846558 in BrowserAction::handleGetLogins (this=0x7f72a0007a10, json=..., action=...) at /home/joerg/git/keepassxc/src/browser/BrowserAction.cpp:260
(More stack frames follow...)
(gdb) p m_wasMinimized
$6 = true

The patch changed

@@ -920,7 +964,7 @@ void BrowserService::databaseUnlocked(DatabaseWidget* dbWidget)
 {
     if (dbWidget) {
         if (m_bringToFrontRequested) {
-            getMainWindow()->lower();
+            hideWindow();
             m_bringToFrontRequested = false;
         }
         emit databaseUnlocked();

Due to m_wasMinimized is true in this case, the patch effectively replace getMainWindow()->lower() by getMainWindow()->showMinimized().

@varjolintu
Copy link
Member

@jo-so There are some changes in the release/2.4.0 branch. We need to make a new fix there if this bug is still valid.

@jo-so
Copy link
Author

jo-so commented Feb 9, 2019 via email

@varjolintu
Copy link
Member

@jo-so The fix is in release/2.4.0 branch. Please see #2667.

@jo-so
Copy link
Author

jo-so commented Feb 9, 2019 via email

@droidmonkey
Copy link
Member

Sweet. You could also just checkout the release/2.4.0 branch

@droidmonkey droidmonkey added this to the v2.4.0 milestone Feb 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants