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

Switch Browser Integration to use native raising of windows in macOS #1904

Merged
merged 1 commit into from Dec 24, 2018

Conversation

Projects
None yet
4 participants
@varjolintu
Copy link
Member

commented May 5, 2018

When popup dialog is launched from KeePassXC-Browser action, the window is raised using native methods (similar to Auto-Type).

Description

A new class MacUtils is added. This class is used only with macOS and it enables few Cocoa functions for bringing a dialog to the front and switching back to the previous application window. It is used with both Auto-Type and Browser Integration.

Also, the other popup dialogs are brough to front, and the KeePassXC main window is lowered/minimized if needed.

Motivation and context

Allows popup dialogs to have a proper focus with macOS.
Fixes #2256.
Fixes #1954.

How has this been tested?

Manually.

Types of changes

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

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]

@varjolintu varjolintu added this to the v2.4.0 milestone May 5, 2018

@varjolintu varjolintu requested a review from keepassxreboot/core-developers May 5, 2018

@varjolintu varjolintu force-pushed the varjolintu:unlock_dialog branch from e8e0f41 to a5be153 May 5, 2018

@weslly weslly self-requested a review May 5, 2018

@varjolintu

This comment has been minimized.

Copy link
Member Author

commented May 5, 2018

Building without browser integration is still broken, so this is WIP.

@varjolintu varjolintu changed the title Use UnlockDatabaseDialog when unlock is requested by KeePassXC-Browser [WIP] Use UnlockDatabaseDialog when unlock is requested by KeePassXC-Browser May 5, 2018

@varjolintu varjolintu force-pushed the varjolintu:unlock_dialog branch from 845cf44 to cbc2028 May 5, 2018

@varjolintu varjolintu changed the title [WIP] Use UnlockDatabaseDialog when unlock is requested by KeePassXC-Browser Use UnlockDatabaseDialog when unlock is requested by KeePassXC-Browser May 5, 2018

@varjolintu

This comment has been minimized.

Copy link
Member Author

commented May 5, 2018

Fixed. Needed to add the BrowserUtils class to src/CMakeLists.txt with the correct linking to the AppKit framework.

@droidmonkey

This comment has been minimized.

Copy link
Member

commented Nov 29, 2018

This needs to be completely redone now that the database widget refactor has occurred. I am going to close this one since its likely a new approach will be needed.

@droidmonkey droidmonkey removed this from the v2.4.0 milestone Nov 29, 2018

@varjolintu

This comment has been minimized.

Copy link
Member Author

commented Nov 29, 2018

@droidmonkey I just made some changes to this PR after the database widget refactor was merged. So no need to close this.

@droidmonkey droidmonkey reopened this Nov 29, 2018

@droidmonkey droidmonkey added this to the v2.4.0 milestone Nov 29, 2018

@varjolintu varjolintu force-pushed the varjolintu:unlock_dialog branch 3 times, most recently from 0b7f750 to 2e95bc8 Nov 29, 2018

@varjolintu varjolintu force-pushed the varjolintu:unlock_dialog branch from 05a9a8f to 7359cf0 Dec 9, 2018

@varjolintu

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2018

I decided to modify this PR to include only the necessary macOS side changes, for now. It's still a step forward. Previously macOS popups didn't even have a proper focus when called.

@varjolintu varjolintu added this to the v2.4.0 milestone Dec 9, 2018

Show resolved Hide resolved src/CMakeLists.txt Outdated

@varjolintu varjolintu force-pushed the varjolintu:unlock_dialog branch from 7359cf0 to 1564063 Dec 9, 2018

Show resolved Hide resolved src/gui/macutils/MacUtils.cpp Outdated

@varjolintu varjolintu force-pushed the varjolintu:unlock_dialog branch from 1564063 to 93369fd Dec 10, 2018

@varjolintu varjolintu force-pushed the varjolintu:unlock_dialog branch from 93369fd to d2f6d91 Dec 20, 2018

@varjolintu

This comment has been minimized.

Copy link
Member Author

commented Dec 20, 2018

Added an Action enum to MessageBox class that allows raising the popup.

Changes were made

@droidmonkey droidmonkey merged commit 5488f1b into keepassxreboot:develop Dec 24, 2018

4 checks passed

CodeFactor No issues found.
Details
MacOS (KeepassXC) TeamCity build finished
Details
Ubuntu Linux (KeepassXC) TeamCity build finished
Details
Windows 10 (KeepassXC) TeamCity build finished
Details

@varjolintu varjolintu deleted the varjolintu:unlock_dialog branch Dec 25, 2018

droidmonkey added a commit that referenced this pull request Mar 19, 2019

Release 2.4.0
- New Database Wizard [#1952]
- Advanced Search [#1797]
- Automatic update checker [#2648]
- KeeShare database synchronization [#2109, #1992, #2738, #2742, #2746, #2739]
- Improve favicon fetching; transition to Duck-Duck-Go [#2795, #2011, #2439]
- Remove KeePassHttp support [#1752]
- CLI: output info to stderr for easier scripting [#2558]
- CLI: Add --quiet option [#2507]
- CLI: Add create command [#2540]
- CLI: Add recursive listing of entries [#2345]
- CLI: Fix stdin/stdout encoding on Windows [#2425]
- SSH Agent: Support OpenSSH for Windows [#1994]
- macOS: TouchID Quick Unlock [#1851]
- macOS: Multiple improvements; include CLI in DMG [#2165, #2331, #2583]
- Linux: Prevent Klipper from storing secrets in clipboard [#1969]
- Linux: Use polling based file watching for NFS [#2171]
- Linux: Enable use of browser plugin in Snap build [#2802]
- TOTP QR Code Generator [#1167]
- High-DPI Scaling for 4k screens [#2404]
- Make keyboard shortcuts more consistent [#2431]
- Warn user if deleting referenced entries [#1744]
- Allow toolbar to be hidden and repositioned [#1819, #2357]
- Increase max allowed database timeout to 12 hours [#2173]
- Password generator uses existing password length by default [#2318]
- Improve alert message box button labels [#2376]
- Show message when a database merge makes no changes [#2551]
- Browser Integration Enhancements [#1497, #2253, #1904, #2232, #1850, #2218, #2391, #2396, #2542, #2622, #2637, #2790]
- Overall Code Improvements [#2316, #2284, #2351, #2402, #2410, #2419, #2422, #2443, #2491, #2506, #2610, #2667, #2709, #2731]
@l0f4r0

This comment has been minimized.

Copy link

commented Mar 30, 2019

In relation to bug #2547, this PR doesn't fix the issue from my side.

@varjolintu

This comment has been minimized.

Copy link
Member Author

commented Mar 30, 2019

@l0f4r0 Could you elaborate and provide some details, thanks.

@l0f4r0

This comment has been minimized.

Copy link

commented Mar 30, 2019

@l0f4r0 Could you elaborate and provide some details, thanks.

Sure. When launching autotyping while database is locked, KeepassXC asks for the db password but is then unable to trigger the correct entry. So actually, you need to unlock the database first and then launch autotyping feature.

I'm using KeepassXC 2.4.0 on macOS 10.14.4.

@droidmonkey

This comment has been minimized.

Copy link
Member

commented Mar 30, 2019

Yeah that bug wasn't actually fixed yet

@l0f4r0

This comment has been minimized.

Copy link

commented Mar 30, 2019

Yeah that bug wasn't actually fixed yet

Ok, no problem. I just thought it would be because of previous statuses on #2547 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.