Show URL field for all schemes.

[rominf]

Description

Show URL field for all schemes.

Motivation and context

Fix #1424

How has this been tested?

Manually.

Screenshots (if appropriate):

Types of changes

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

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]

[phoerious]

The tests are failing.

[rominf]

@phoerious Sorry. I fixed tests.

[TheZ3ro]

This change isn't complete as-is. Removing the check will allow ftp, cmd and other URLs in DetailsWidget and EntryModel as clickable links.

So, you removed the check from the Entry::resolveUrl function but you need to add that same check in the Entry::displayUrl function (that is calling resolveUrl)

Another thing I noticed is EntryModel (line 182) resolving reference for Entry::displayUrl but there will be no reference in it (displayUrl already resolve them).
Can you change that line with simply result = entry->displayUrl(); ?

Thanks for the contribution

[rominf]

@TheZ3ro

This change isn't complete as-is. Removing the check will allow ftp, cmd and other URLs in DetailsWidget and EntryModel as clickable links.

What's wrong with that? When I click on a ftp:// link, keepassxc opens Dolphin as an FTP client. That's the correct behavior in my opinion.

but you need to add that same check in the Entry::displayUrl function (that is calling resolveUrl)

Why?

[phoerious]

You should also get into the habit of using speaking PR titles and commit messages. One should be able to read them without having to look up what #1424 was about.

[TheZ3ro]

Now I remember why we put those checks in the first place.
The favicon downloader (EditEntryWidget.cpp#L631 and EditWidgetIcons.cpp#L189 ) was crashing when a non-http link was inserted.
So we created the webUrl() function for http-only urls and the displayUrl() function for masking passwords in URLs.

Now with curl the favicon downloader doesn't seems to crash anymore (I tested only with an ftp url) so I think we are safe after removing the http/https check

[TheZ3ro]

This can be merged in 2.3.1

[tycho]

IMO, this needs a better commit message than just "Fix #1424"...

[droidmonkey]

Closed in favor of #1768