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

Launch KeePassXC password generator popup from the extension #6529

Conversation

varjolintu
Copy link
Member

@varjolintu varjolintu commented May 16, 2021

Add support for launching the password generator popup from the browser and returning the generated password.
Removes all password related settings from BrowserSettings class.

Related KeePassXC-Browser PR: keepassxreboot/keepassxc-browser#1329

Fixes #6473.

Testing strategy

Manually.

Type of change

  • ✅ New feature (change that adds functionality)

src/browser/BrowserAction.cpp Outdated Show resolved Hide resolved
src/browser/BrowserService.cpp Outdated Show resolved Hide resolved
@varjolintu varjolintu force-pushed the feature/use_password_gen_popup_from_browser branch 2 times, most recently from 06ba242 to c5e1d94 Compare October 11, 2021 17:11
@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2021

Codecov Report

Attention: Patch coverage is 12.82051% with 34 lines in your changes missing coverage. Please review.

Project coverage is 63.93%. Comparing base (9aa30c4) to head (fbd3495).
Report is 586 commits behind head on develop.

Files with missing lines Patch % Lines
src/browser/BrowserService.cpp 0.00% 21 Missing ⚠️
src/browser/BrowserAction.cpp 35.71% 9 Missing ⚠️
src/gui/PasswordGeneratorWidget.cpp 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6529      +/-   ##
===========================================
+ Coverage    63.77%   63.93%   +0.17%     
===========================================
  Files          330      330              
  Lines        41857    41750     -107     
===========================================
+ Hits         26691    26692       +1     
+ Misses       15166    15058     -108     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@varjolintu
Copy link
Member Author

Not working yet, even if I made the fixes. If the password dialog is closed, after that it doesn't fill anymore when launched again.

@varjolintu
Copy link
Member Author

Fixed a bug where the signal was connected multiple times (on every new request). Moved the lambda to the constructor so we can make sure it's only connected once.

@droidmonkey
Copy link
Member

This works REALLY well, great job @varjolintu. As a note, when you connect a lambda function to a signal, you need to provide a context (the third parameter in connect) to ensure the lambda is destroyed when the context is destroyed. I added it to BrowserService and BrowserAction calls for you.

@droidmonkey droidmonkey force-pushed the feature/use_password_gen_popup_from_browser branch from 77b1abb to fbd3495 Compare October 24, 2021 12:51
@droidmonkey droidmonkey merged commit dd41f09 into keepassxreboot:develop Oct 24, 2021
@varjolintu varjolintu deleted the feature/use_password_gen_popup_from_browser branch October 24, 2021 17:37
@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
feature: Browser pr: new feature Pull request that adds a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In-browser password generator does not respect "Also include" saved settings from KeepassXC window
4 participants