Skip to content

Fix Best-Matching ..again#5316

Merged
droidmonkey merged 2 commits intokeepassxreboot:release/2.6.2from
varjolintu:fix/best-matching-again
Sep 13, 2020
Merged

Fix Best-Matching ..again#5316
droidmonkey merged 2 commits intokeepassxreboot:release/2.6.2from
varjolintu:fix/best-matching-again

Conversation

@varjolintu
Copy link
Copy Markdown
Member

@varjolintu varjolintu commented Aug 22, 2020

It was still broken. And actually the change for passing a full URL from the extension fixed the issue without any extra handling in the BrowserService.cpp.

Fixes #5299.

Testing strategy

Added some more tests, and tested it also manually. Double verification will be needed. This should be the final fix for this. Really.

Type of change

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

@varjolintu varjolintu added the bug label Aug 22, 2020
@varjolintu varjolintu added this to the v2.6.2 milestone Aug 22, 2020
@varjolintu varjolintu force-pushed the fix/best-matching-again branch from 41fd664 to f2156da Compare August 22, 2020 11:48
@droidmonkey
Copy link
Copy Markdown
Member

The whole logic tree needs a refactor. I suspect we are also not handling multiple urls correctly.

@varjolintu
Copy link
Copy Markdown
Member Author

@droidmonkey That needs to be checked too.

@varjolintu
Copy link
Copy Markdown
Member Author

varjolintu commented Aug 23, 2020

Tested the Additional URL's behavior with the following scenario:

  • Three entries with URL's https://github.com/loginpage, https://test.github.com/ and https://github.com/
  • Added Additional URL https://test.github.com/anotherpage to the first entry
  • Requested URL https://test.github.com/anotherpage
  • Sorting only returns the second entry as a priority. It should return the first one instead because the URL matches better.

EDIT: And of course, I'm making a test case.

EDIT 2: Made a change inside BrowserService::sortPriority() that also retrieves the sorting number for Additional URL's in the entry, stores all in a list (including entry's own URL) and returns the max value. It works quite well.

@droidmonkey
Copy link
Copy Markdown
Member

Is this ready for review?

@varjolintu
Copy link
Copy Markdown
Member Author

@droidmonkey Yes.

droidmonkey pushed a commit to varjolintu/keepassxc that referenced this pull request Aug 31, 2020
@droidmonkey droidmonkey force-pushed the fix/best-matching-again branch from b942172 to 22e59f3 Compare August 31, 2020 17:21
@droidmonkey droidmonkey changed the base branch from develop to release/2.6.2 August 31, 2020 17:21
* Handle Additional URLs when doing priority sorting
* Clean up function calls
* Fixes keepassxreboot#5299
@droidmonkey droidmonkey force-pushed the fix/best-matching-again branch 3 times, most recently from 3a38703 to 421f499 Compare September 6, 2020 22:00
@droidmonkey droidmonkey force-pushed the fix/best-matching-again branch from 421f499 to 577bbce Compare September 12, 2020 20:39
Copy link
Copy Markdown
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.

Fixed remaining issues with Qt < 5.11

@droidmonkey droidmonkey merged commit e391dd1 into keepassxreboot:release/2.6.2 Sep 13, 2020
@varjolintu varjolintu deleted the fix/best-matching-again branch September 13, 2020 17:08
phoerious added a commit that referenced this pull request Oct 21, 2020
Added

- Add option to keep window always on top to view menu [#5542]
- Move show/hide usernames and passwords to view menu [#5542]
- Add command line options and environment variables for changing the config locations [#5452]
- Include TOTP settings in CSV import/export and add support for ISO datetimes [#5346]

Changed

- Mask sensitive information in command execution confirmation prompt [#5542]
- SSH Agent: Avoid shortcut conflict on macOS by changing "Add key" to Ctrl+H on all platforms [#5484]

Fixed

- Prevent data loss with drag and drop between databases [#5536]
- Fix crash when toggling Capslock rapidly [#5545]
- Don't mark URL references as invalid URL [#5380]
- Reset entry preview after search [#5483]
- Set Qt::Dialog flag on database open dialog [#5356]
- Fix sorting of database report columns [#5426]
- Fix IfDevice matching logic [#5344]
- Fix layout issues and a stray scrollbar appearing on top of the entry edit screen [#5424]
- Fix tabbing into the notes field [#5424]
- Fix password generator ignoring settings on load [#5340]
- Restore natural entry sort order on application load [#5438]
- Fix paperclip and TOTP columns not saving state [#5327]
- Enforce fixed password font in entry preview [#5454]
- Add scrollbar when new database wizard exceeds screen size [#5560]
- Do not mark database as modified when viewing Auto-Type associations [#5542]
- CLI: Fix two heap-use-after-free crashes [#5368,#5470]
- Browser: Fix key exchange not working with multiple simultaneous users on Windows [#5485]
- Browser: Fix entry retrieval when "only best matching" is enabled [#5316]
- Browser: Ignore recycle bin on KeePassHTTP migration [#5481]
- KeeShare: Fix import crash [#5542]
- macOS: Fix toolbar theming and breadcrumb display issues [#5482]
- macOS: Fix file dialog randomly closing [#5479]
- macOS: Fix being unable to select OPVault files for import [#5341]
@phoerious phoerious added pr: bugfix Pull request fixes a bug and removed bug 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: bugfix Pull request fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2.6.1: browser does not find entry

4 participants