Skip to content

Show UI warning for invalid URL's#3912

Merged
droidmonkey merged 1 commit intokeepassxreboot:release/2.5.2from
varjolintu:hotfix/browser_url_ui_validation
Dec 21, 2019
Merged

Show UI warning for invalid URL's#3912
droidmonkey merged 1 commit intokeepassxreboot:release/2.5.2from
varjolintu:hotfix/browser_url_ui_validation

Conversation

@varjolintu
Copy link
Copy Markdown
Member

@varjolintu varjolintu commented Nov 22, 2019

Type of change

  • ✅ New feature (non-breaking change which adds functionality)

Description and Context

Shows a warning when editing entry if the URL field has an invalid value.

Also works with Additional URL's page, although the background color only changes after editing.

Screenshots

Valid URL:
Screen Shot 2019-11-22 at 13 56 59

Invalid URL:
Screen Shot 2019-11-22 at 13 57 09

Invalid URL in Additional URL's page:
Screen Shot 2019-11-22 at 23 17 21

Testing strategy

Tests added to TestBrowser class.

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]
  • ✅ I have added tests to cover my changes.

@varjolintu varjolintu added this to the v2.5.2 milestone Nov 22, 2019
@varjolintu varjolintu requested review from droidmonkey and phoerious and removed request for droidmonkey November 22, 2019 12:51
@varjolintu varjolintu force-pushed the hotfix/browser_url_ui_validation branch from 9ac0815 to b2723ec Compare November 22, 2019 12:52
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.

See comment

@droidmonkey
Copy link
Copy Markdown
Member

Are we going to extend this to the multi url list or wait for 2.6.0?

@varjolintu
Copy link
Copy Markdown
Member Author

@droidmonkey I'll test it today and see if it's possible to do. Should be a small fix.

@varjolintu
Copy link
Copy Markdown
Member Author

@droidmonkey It was possible. Although only the background color changes after editing. QModelIndex has its limits.

@droidmonkey
Copy link
Copy Markdown
Member

You can show an icon in the QModelIndex, that is how I show the paperclip on the entry view model.

@varjolintu
Copy link
Copy Markdown
Member Author

@droidmonkey Oh. Didn't know that. I'll add the icon.

@varjolintu
Copy link
Copy Markdown
Member Author

Added the icon to the additional URL field's right side + removed the obsolete code from URLEdit class.

Thanks for the feedback everyone!

@varjolintu varjolintu force-pushed the hotfix/browser_url_ui_validation branch 2 times, most recently from faaca1b to 47d5cd5 Compare November 26, 2019 16:30
@varjolintu
Copy link
Copy Markdown
Member Author

Removed the extra check, now tests with Linux are also fine. Everything's ready if there's nothing more to add.

@varjolintu varjolintu force-pushed the hotfix/browser_url_ui_validation branch from 47d5cd5 to 361dd56 Compare December 6, 2019 10:38
@droidmonkey droidmonkey force-pushed the hotfix/browser_url_ui_validation branch from 361dd56 to 5a630cb Compare December 21, 2019 03:13
@droidmonkey droidmonkey merged commit c0f29cc into keepassxreboot:release/2.5.2 Dec 21, 2019
@varjolintu varjolintu deleted the hotfix/browser_url_ui_validation branch December 21, 2019 20:14
droidmonkey added a commit that referenced this pull request Jan 4, 2020
Added

- Browser: Show UI warning when entering invalid URLs [#3912]
- Browser: Option to use an entry only for HTTP auth [#3927]

Changed

- Disable the user interface when merging or saving the database [#3991]
- Ability to hide protected attribute after reveal [#3877]
- Remove mention of "snaps" in Windows and macOS [#3879]
- CLI: Merge parameter for source database key file (--key-file-from) [#3961]
- Improve GUI tests reliability on Hi-DPI displays [#4075]
- Disable deprecation warnings to allow building with Qt 5.14+ [#4075]
- OPVault: Use 'otp' attribute for TOTP field imports [#4075]

Fixed

- Fix crashes when saving a database to cloud storage [#3991]
- Fix crash when pressing enter twice while opening database [#3885]
- Fix handling of HTML when displayed in the entry preview panel [#3910]
- Fix start minimized to tray on Linux [#3899]
- Fix Auto Open with key file only databases [#4075]
- Fix escape key closing the standalone password generator [#3892]
- macOS: Fix monospace font usage in password field and notes [#4075]
- macOS: Fix building on macOS 10.9 to 10.11 [#3946]
- Fix TOTP setup dialog not closing on database lock [#4075]
- Browser: Fix condition where additional URLs are ignored [#4033]
- Browser: Fix subdomain matching to return only relevant site entries [#3854]
- Secret Service: Fix multiple crashes and incompatibilities [#3871, #4009, #4074]
- Secret Service: Fix searching of entries [#4008, #4036]
- Secret Service: Fix behavior when exposed group is recycled [#3914]
- CLI: Release the database instance before exiting interactive mode [#3889]
- Fix (most) memory leaks in tests [#3922]
@phoerious phoerious added pr: new feature Pull request 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

pr: new feature Pull request adds a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants