Fix "Invalid URL: QUrl("some.txt")" warnings in Save dialog
ClosedPublic

Authored by Lekensteyn on Feb 15 2019, 1:46 AM.

Details

Summary

When the Save File dialog is in use with filters, any relative path
would trigger warnings due to KIO::stat receiving an invalid URL in
KFileWidgetPrivate::updateLocationEditExtension. Fix the warning by
ensuring that relative paths are valid file URLs.

Add a bunch of tests to check the expected properties while at it. These
tests pass regardless of the fix but after the fix, warnings are gone.

BUG: 373119

Test Plan

Before the kfilewidget.cpp change:

$ ninja bin/kfilewidgettest && bin/kfilewidgettest testFilterChange testSetFilterForSave
Config: Using QtTest library 5.12.0, Qt 5.12.0 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 8.2.1 20181127)
PASS   : KFileWidgetTest::initTestCase()
QWARN  : KFileWidgetTest::testFilterChange() kf5.kio.core: Invalid URL: QUrl("some.txt")
QWARN  : KFileWidgetTest::testFilterChange() kf5.kio.core: Invalid URL: QUrl("some.txt")
QWARN  : KFileWidgetTest::testFilterChange() kf5.kio.core: Invalid URL: QUrl("some.txt")
Qt: Session management error: networkIdsList argument is NULL
PASS   : KFileWidgetTest::testFilterChange()
QWARN  : KFileWidgetTest::testSetFilterForSave(some.txt) kf5.kio.core: Invalid URL: QUrl("some.txt")
QWARN  : KFileWidgetTest::testSetFilterForSave(some.txt) kf5.kio.core: Invalid URL: QUrl("some.txt")
PASS   : KFileWidgetTest::testSetFilterForSave(some.txt)
PASS   : KFileWidgetTest::testSetFilterForSave(extensionless name)
PASS   : KFileWidgetTest::testSetFilterForSave(existing file)
QWARN  : KFileWidgetTest::testSetFilterForSave(some.2019) kf5.kio.core: Invalid URL: QUrl("some.2019")
QWARN  : KFileWidgetTest::testSetFilterForSave(some.2019) kf5.kio.core: Invalid URL: QUrl("some.txt")
PASS   : KFileWidgetTest::testSetFilterForSave(some.2019)
QWARN  : KFileWidgetTest::testSetFilterForSave(some.html) kf5.kio.core: Invalid URL: QUrl("some.html")
QWARN  : KFileWidgetTest::testSetFilterForSave(some.html) kf5.kio.core: Invalid URL: QUrl("some.txt")
PASS   : KFileWidgetTest::testSetFilterForSave(some.html)
PASS   : KFileWidgetTest::cleanupTestCase()

After the change, all QWARN lines are gone.

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Lekensteyn requested review of this revision.Feb 15 2019, 1:46 AM
Lekensteyn created this revision.
fvogt added a comment.Feb 15 2019, 7:52 AM

Can you add context to this diff by either using arc or git diff with more context?

Lekensteyn updated this revision to Diff 51736.Feb 15 2019, 9:44 AM

Added context, I thought that it would be looked up by Phabricator once linked to a repo.

ngraham accepted this revision.Feb 15 2019, 4:38 PM
ngraham added a reviewer: Frameworks.

Looks good to me, but please wait for approval by @dfaure or another Frameworks person before landing.

This revision is now accepted and ready to land.Feb 15 2019, 4:38 PM
dfaure requested changes to this revision.Mar 3 2019, 8:48 AM

oops sorry forgot to submit my comments earlier

src/filewidgets/kfilewidget.cpp
792–798

This comment is no longer true, is it?
We won't keep it relative, we'll instead make it absolute, just below.

794

That's not what the code does, is it?
relativeUrlTest is relative to ops->url(), which might not be local.

The code looks fine, the comment looks wrong and confusing.

This revision now requires changes to proceed.Mar 3 2019, 8:48 AM
Lekensteyn updated this revision to Diff 53066.Mar 3 2019, 3:49 PM
Lekensteyn marked 2 inline comments as done.

@dfaure I've replaced the outdated comment, how does this version look?

dfaure accepted this revision.Mar 3 2019, 7:42 PM
This revision is now accepted and ready to land.Mar 3 2019, 7:42 PM
This revision was automatically updated to reflect the committed changes.