Add unlock dialog on Autotype and show default Autotype sequence#89
Add unlock dialog on Autotype and show default Autotype sequence#89droidmonkey merged 8 commits intodevelopfrom
Conversation
|
Seems that Travis don't like the second commit. I need to fix that |
|
There is a small issue with this PR. @droidmonkey What can we do? |
src/core/Group.cpp
Outdated
|
|
||
| const Group* group = this; | ||
| // check if group exist (if the entry has a group) | ||
| if(group) { |
There was a problem hiding this comment.
This check doesn't make sense, "this" cannot be null unless you are calling a null pointer to begin with. This check needs to go in the function that calls into the group class.
src/core/Entry.cpp
Outdated
| if (!m_data.defaultAutoTypeSequence.isEmpty()) { | ||
| return m_data.defaultAutoTypeSequence; | ||
| } | ||
| QString sequence = group()->effectiveAutoTypeSequence(); |
There was a problem hiding this comment.
Thanks is where the group null check should go.
|
Fixed. What about the multiple database behavior ? |
src/core/Entry.cpp
Outdated
| if(grp) { | ||
| sequence = grp->effectiveAutoTypeSequence(); | ||
| } else { | ||
| sequence = QString(); |
There was a problem hiding this comment.
Technically this is unnecessary, but not wrong.
src/core/Group.cpp
Outdated
|
|
||
| const Group* group = this; | ||
| do { | ||
| if (!enableSet) { |
There was a problem hiding this comment.
I'll be honest the way this is coded is very confusing. The while conditions seem like they should be satisfied upfront instead of lumbering through the loop with a return statement embedded in there. Recommend cleaning that up.
There was a problem hiding this comment.
This is the code that directly come from src/autotype/Autotype.cpp -> AutoType::autoTypeSequence
I will do a cleanup (maybe also in the autoTypeSequence function?)
|
Ping @droidmonkey |
|
Everything seems to work fine, except for when you create a NEW group/entry it does not populate with the default auto-type sequence. This is obviously related to the null check. I think we need to open a separate issue for that particular bug. |
|
Yeah, the issue is in the |
There is a small issue with this PR. @droidmonkey
If multiple databases are loaded: first check if there is an unlocked one, if not ask the password for the first locked database.
Do we want a Dialog for select witch DB to unlock? (This needs 4 new file, 2 for the dialog, 2 for the view)
Description
Motivation and Context
Fixes #10
Based on keepassx/keepassx#160 by @andrewcchen
Based on keepassx/keepassx#103 by @cbrunet
How Has This Been Tested?
Manually, with
make test(all test are passed), and with TravisCIScreenshots (if appropriate):
Sorry for the italian language in the first screenshot
Types of changes
Checklist: