Skip to content

Conversation

@droidmonkey
Copy link
Member

@droidmonkey droidmonkey commented May 17, 2025

Testing strategy

Tested on Windows

Type of change

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

@droidmonkey droidmonkey added this to the v2.7.11 milestone May 17, 2025
@droidmonkey droidmonkey added the pr: bugfix Pull request fixes a bug label May 17, 2025

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented May 17, 2025

Codecov Report

Attention: Patch coverage is 65.04854% with 36 lines in your changes missing coverage. Please review.

Project coverage is 64.24%. Comparing base (c4b4be4) to head (dd0574c).
Report is 9 commits behind head on develop.

Files with missing lines Patch % Lines
src/gui/PasswordGeneratorWidget.cpp 44.44% 10 Missing ⚠️
src/gui/MainWindow.cpp 18.18% 9 Missing ⚠️
src/autotype/AutoType.cpp 0.00% 7 Missing ⚠️
src/core/Totp.cpp 81.25% 3 Missing ⚠️
src/gui/MessageBox.cpp 0.00% 2 Missing ⚠️
src/autotype/AutoTypeSelectDialog.cpp 0.00% 1 Missing ⚠️
src/browser/BrowserService.cpp 0.00% 1 Missing ⚠️
src/core/Entry.cpp 83.33% 1 Missing ⚠️
src/gui/DatabaseWidget.cpp 50.00% 1 Missing ⚠️
src/gui/EntryPreviewWidget.cpp 93.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12102      +/-   ##
===========================================
+ Coverage    63.89%   64.24%   +0.35%     
===========================================
  Files          369      375       +6     
  Lines        38943    39292     +349     
===========================================
+ Hits         24882    25241     +359     
+ Misses       14061    14051      -10     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xboxones1

This comment was marked as outdated.

@droidmonkey

This comment was marked as outdated.

@varjolintu

This comment was marked as outdated.

@droidmonkey
Copy link
Member Author

Ok got both issues fixed:

image

Also properly hiding the progress bar and text label if the TOTP code isn't valid on the popup dialog.

@varjolintu
Copy link
Member

Maybe the copy button should be disabled also if the TOTP is not valid? After all it's just copying the "Invalid Key" value to clipboard.

@droidmonkey
Copy link
Member Author

droidmonkey commented May 18, 2025

Added another fix for the passphrase generator wordlist size handling:

image

I also added disabling the copy button on the TOTP dialog if the code is invalid

Copy link
Member

@varjolintu varjolintu left a comment

Choose a reason for hiding this comment

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

Now when the Copy button is disabled in the dialog, it would make sense also disable the Copy TOTP and Copy Password and TOTP menu items when TOTP is invalid. Not mandatory though.

@xboxones1
Copy link
Contributor

xboxones1 commented May 18, 2025

And show qr-code can also be disabled

@droidmonkey
Copy link
Member Author

droidmonkey commented May 18, 2025

The buttons are only testing if totp is configured... that would be a bigger change to the code logic deep in the totp settings. I will consider this.

@droidmonkey
Copy link
Member Author

droidmonkey commented May 18, 2025

Ok well this turned out much easier than I expected and actually improves a lot of behaviors:

image

image

Give this a thorough test, but I think I hit everything, even copying TOTP from the auto-type dialog and browser extension

@dqnk

This comment was marked as outdated.

@droidmonkey

This comment was marked as outdated.

@xboxones1

This comment was marked as outdated.

* Fixes #11808
* Don't show tear off menu or option to tear off if there are no tags
* Fix "No Tags" not being shown on first hover
* Fix issues when using a tag named "No Tags"
* Fix #12153 - tags becoming unsorted in the context menu when switching between database tabs
* Fixes #11662 - disable the save button when viewing Password Generator and Application Settings to restore previous behavior of toolbar
* Fixes #11856
* Set the minimum recommended wordlist size to 1,296 - equal to the EFF Short List
* Issue a clear warning when using a smaller wordlist but do not prevent generation of passphrases
* Improve wording when removing custom wordlist
* Fixes #11357
* Introduces validity parameter to TOTP generator function for future use elsewhere in the code base
* Fixes this in preview panel and TOTP dialog
* Disable actions to copy/show TOTP if the settings are invalid
* Show an error message on the TOTP setup dialog if the settings are invalid
* Show a TOTP icon with an x if the settings are invalid
* Keeps readability of translation strings without losing line breaks due to forced rich text display
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses several minor UI, functionality, and data-handling issues across the application:

  • Hides or disables UI elements when no tags or invalid TOTP settings are present and restores toolbar save button behavior
  • Refactors the TOTP API to include a validity flag with corresponding codepaths in entries, database, autotype, and browser services
  • Updates passphrase generator’s minimum wordlist size, renames validation methods, and surfaces warnings without blocking generation
  • Cleans up start menu shortcuts on uninstall and sorts tags using a collator

Reviewed Changes

Copilot reviewed 20 out of 35 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/gui/EntryPreviewWidget.ui Wrapped TOTP label/progress in a zero-margin QWidget container
src/gui/EntryPreviewWidget.cpp Connected toggle to container visibility; unified show/hide logic
src/gui/EditWidget.cpp Expanded pageIndex to consider the scroll area itself
src/gui/DatabaseWidget.cpp Switched from hasTotp() to hasValidTotp() for clipboards
src/fdosecrets/objects/Item.cpp Updated TOTP check to hasValidTotp()
src/core/Totp.h & src/core/Totp.cpp Added checkValidSettings(), extended generateTotp() with flag
src/core/PassphraseGenerator.h & .cpp Renamed isValid() to isWordListValid(), adjusted min size
src/core/Entry.h & src/core/Entry.cpp Modified totp() signature and introduced hasValidTotp()
src/core/Database.cpp Replaced m_tagList.sort() with std::sort() + QCollator
src/cli/Diceware.cpp Downgraded wordlist-too-small error to a warning
src/browser/BrowserService.cpp Guarded JSON TOTP inclusion with hasValidTotp()
src/autotype/AutoTypeSelectDialog.cpp Switched to hasValidTotp() for action menu updates
src/autotype/AutoType.cpp Added explicit invalid-TOTP error path in sequence parsing
share/windows/wix-template.xml Added <RemoveFile> to clear start menu shortcuts on uninstall
share/translations/keepassxc_en.ts Added new translations; removed obsolete entries
share/icons/icons.qrc & COPYING Added totp-invalid.svg and updated license manifest
Comments suppressed due to low confidence (4)

share/translations/keepassxc_en.ts:727

  • Missing translation entry for the "Invalid Settings" error returned by Totp::checkValidSettings(). Add a <message> block for this source string under the appropriate context.
    <message>

share/translations/keepassxc_en.ts:727

  • Missing translation entry for the "Invalid Key" error returned by Totp::checkValidSettings(). Add a <message> block for this source string under the TOTP context.
    <message>

src/core/PassphraseGenerator.h:53

  • [nitpick] Member variable m_minWordListSize deviates from the repository’s snake_case convention (e.g., m_minimum_wordlist_length). Consider renaming to m_minimum_word_list_size for consistency.
    int m_minWordListSize = 1296;

src/core/PassphraseGenerator.cpp:120

  • The original check for m_wordCount > 0 was removed. Without validating m_wordCount, you may generate zero-word passphrases. Reintroduce a check on m_wordCount or ensure it’s always positive.
    if (m_wordlist.isEmpty()) {

@droidmonkey droidmonkey merged commit 6407893 into develop Jun 19, 2025
12 checks passed
@droidmonkey droidmonkey deleted the fix/minor-fixes branch June 19, 2025 20:22
@droidmonkey droidmonkey added the pr: backported Pull request backported to previous release label Jun 19, 2025
@xboxones1
Copy link
Contributor

@droidmonkey, I checked and it still hasn't been fixed. Or don't you see what the problem is?

@droidmonkey
Copy link
Member Author

What hasn't been fixed? I could not replicate your gif

@xboxones1
Copy link
Contributor

xboxones1 commented Jun 20, 2025

@droidmonkey, I outlined the steps to reproduce the issue, but you marked the message as outdated. I thought a GIF would be more helpful.

  1. In the security settings, disable the option to hide TOTP in the preview panel.
  2. Expand the preview panel so that it is visible after unlocking the database.
  3. After unlocking, ensure that TOTP is configured in the first entrey that is opened.
  4. After unlocking the database, TOTP should display in the preview panel, but it does not. You have to click twice, as shown in the GIF. This is the bug.
  5. This issue occurs on Linux, but I can build on Windows to test

@droidmonkey
Copy link
Member Author

Ah, I figured it out, fix inbound

dragonekii pushed a commit to dragonekii/keepassxc-custom that referenced this pull request Dec 8, 2025
Release 2.7.11 (2025-11-23)

- Add image, HTML, Markdown preview, and text editing support to inline attachment viewer [keepassxreboot#12085, keepassxreboot#12244, keepassxreboot#12654]
- Add database merge confirmation dialog [keepassxreboot#10173]
- Add option to auto-generate a password for new entries [keepassxreboot#12593]
- Add support for group sync in KeeShare [keepassxreboot#11593]
- Add {UUID} placeholder for use in references [keepassxreboot#12511]
- Add “Wait for Enter” search option [keepassxreboot#12263]
- Add keyboard shortcut to “Jump to Group” from search results [keepassxreboot#12225]
- Add predefined search for TOTP entries [keepassxreboot#12199]
- Add confirmation when closing database via ESC key [keepassxreboot#11963]
- Add support for escaping placeholder expressions [keepassxreboot#11904]
- Reduce tab indentation width in notes fields [keepassxreboot#11919]
- Cap default Argon2 parallelism when creating a new database [keepassxreboot#11853]
- Database lock after inactivity now enabled by default and set to 900 seconds [keepassxreboot#12689, keepassxreboot#12609]
- Copying TOTP now opens setup dialog if none is configured for entry [keepassxreboot#12584]
- Make double click action configurable [keepassxreboot#12322]
- Remove unused “Last Accessed” from GUI [keepassxreboot#12602]
- Auto-Type: Add more granular confirmation settings [keepassxreboot#12370]
- Auto-Type: Add URL typing preset and add copy options to menu [keepassxreboot#12341]
- Browser: Do not allow sites automatically if entry added from browser extension [keepassxreboot#12413]
- Browser: Add options to restrict exposed groups [keepassxreboot#9852, keepassxreboot#12119]
- Bitwarden Import: Add support for timestamps and password history [keepassxreboot#12588]
- macOS: Add Liquid Glass icon [keepassxreboot#12642]
- macOS: Remove theme-based menubar icon toggle [keepassxreboot#12685]
- macOS: Add Window and Help menus [keepassxreboot#12357]
- Windows: Add option to add KeePassXC to PATH during installation [keepassxreboot#12171]

- Fix window geometry not being restored properly when KeePassXC starts in tray [keepassxreboot#12683]
- Fix potential database truncation when using direct write save method with YubiKeys [keepassxreboot#11841]
- Fix issue with database backup saving [keepassxreboot#11874]
- Fix UI lockups during startup with multiple tabs [keepassxreboot#12053]
- Fix keyboard shortcuts when menubar is hidden [keepassxreboot#12431]
- Fix clipboard being cleared on exit even if no password was copied [keepassxreboot#12603]
- Fix single-instance detection when username contains invalid filename characters [keepassxreboot#12559]
- Fix “Search Wait for Enter” setting not being save [keepassxreboot#12614]
- Fix hotkey accelerators not being escaped properly on database tabs [keepassxreboot#12630]
- Fix confusing error if user cancels out of key file edit dialog [keepassxreboot#12639]
- Fix issues with saved searches and “Press Enter to Search” option [keepassxreboot#12314]
- Fix URL wildcard matching [keepassxreboot#12257]
- Fix TOTP visibility on unlock and settings change [keepassxreboot#12220]
- Fix KeeShare entries with reference attributes not updating [keepassxreboot#11809]
- Fix sort order not being maintained when toggling filters in database reports [keepassxreboot#11849]
- Fix several UI font and layout issues [keepassxreboot#11967,  keepassxreboot#12102]
- Prevent mouse wheel scroll on edit username field [keepassxreboot#12398]
- Improve base translation consistency [keepassxreboot#12432]
- Improve inactivity timer [keepassxreboot#12246]
- Documentation improvements [keepassxreboot#12373, keepassxreboot#12506]
- Browser: Fix ordering of clientDataJSON in Passkey response object [keepassxreboot#12120]
- Browser: Fix URL matching for additional URLs [keepassxreboot#12196]
- Browser: Fix group settings inheritance [keepassxreboot#12368]
- Browser: Allow read-only native messaging config files [keepassxreboot#12236]
- Browser: Optimise entry iteration in browser access control dialog [keepassxreboot#11817]
- Browser: Fix “Do not ask permission for HTTP Basic Auth” option [keepassxreboot#11871]
- Browser: Fix native messaging path for Tor Browser launcher on Linux [keepassxreboot#12005]
- Auto-Type: Fix empty window behaviour [keepassxreboot#12622]
- Auto-Type: Take delays into account when typing TOTP [keepassxreboot#12691]
- SSH Agent: Fix out-of-memory crash with malformed SSH keys [keepassxreboot#12606]
- CSV Import: Fix modified and creation time import [keepassxreboot#12379]
- CSV Import: Fix duplication of root groups on import [keepassxreboot#12240]
- Proton Pass Import: Fix email addresses not being imported when no username set [keepassxreboot#11888]
- macOS: Fix secure input getting stuck [keepassxreboot#11928]
- Windows: Prevent launch as SYSTEM user from MSI installer [keepassxreboot#12705]
- Windows: Remove broken check for MSVC Redistributable from MSI installer [keepassxreboot#11950]
- Linux: Fix startup delay due to StartupNotify setting in desktop file [keepassxreboot#12306]
- Linux: Fix memory initialisation when --pw-stdin is used with a pipe [keepassxreboot#12050]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: backported Pull request backported to previous release pr: bugfix Pull request fixes a bug

Projects

None yet

5 participants