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

Brings messagebox to the front when updating credentials from KeePassXC-Browser #1830

Merged

Conversation

varjolintu
Copy link
Member

When updating credentials from KeePassXC-Browser the QMessageBox is now brought to the front.

Motivation and context

Previously the messagebox was visible behind the browser window.
Fixes #1829.

How has this been tested?

Manually.

Types of changes

  • ✅ Bug fix (non-breaking change which fixes an issue)

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]

Copy link
Member

@droidmonkey droidmonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment

tr("Do you want to update the information in %1 - %2?")
.arg(QUrl(url).host()).arg(username),
QMessageBox::Yes|QMessageBox::No);
QMessageBox msgBox;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we be using the MessageBox class here which reimplements qmessagebox with a wrapper.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. We should.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, now when I looked at the MessageBox implementation it doesn't allow me to do all the necessary things like setting the window flags. That wrapper is just useful for a quick dialog reply. Also, in this case using exec() is necessary.

Copy link
Member

@droidmonkey droidmonkey Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I gotcha. Ok well this looks great then.

@varjolintu varjolintu added this to the v2.3.2 milestone Apr 25, 2018
@droidmonkey droidmonkey merged commit 635d6fe into keepassxreboot:release/2.3.2 May 4, 2018
@varjolintu varjolintu deleted the credentials_update branch May 4, 2018 21:15
droidmonkey added a commit that referenced this pull request May 8, 2018
- Enable high entropy ASLR on Windows [#1747]
- Enhance favicon fetching [#1786]
- Fix crash on Windows due to autotype [#1691]
- Fix dark tray icon changing all icons [#1680]
- Fix --pw-stdin not using getPassword function [#1686]
- Fix placeholders being resolved in notes [#1907]
- Enable auto-type start delay to be configurable [#1908]
- Browser: Fix native messaging reply size [#1719]
- Browser: Increase maximum buffer size [#1720]
- Browser: Enhance usability and functionality [#1810, #1822, #1830, #1884, #1906]
- SSH Agent: Parse aes-256-cbc/ctr keys [#1682]
- SSH Agent: Enhance usability and functionality [#1677, #1679, #1681, #1787]
jtl999 pushed a commit to jtl999/keepassxc that referenced this pull request Jul 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants