D8920: Fixes url navigation with relative links on KUrlNavigator

[David Faure]

dfaure added a comment.


  Sounds to me like ~/foo should always mean $HOME/foo even if that doesn't exist.

INLINE COMMENTS

> kurlnavigatortest.cpp:207
> +
> +    QTest::newRow("relativeFile") << QStringLiteral(".some_dotfile") << QUrl::fromUserInput(".some_dotfile", QLatin1String(""), QUrl::AssumeLocalFile);
> +    QTest::newRow("relativeDir") << QStringLiteral("some_directory") << QUrl::fromUserInput("some_directory", QLatin1String(""), QUrl::AssumeLocalFile);

I think this should have a clear expected value, not the same call as the implementation, which is then not testing the actual behavior; it's like checking that a==a, but not the value of a.

I mean, to me it's unclear what fromUserInput(3 args) does when the 2nd arg is QLatin1String("") (yes, even though I actually wrote that method!).
Why QLatin1String("")? Does it behave any different when called with QString() instead? In any case, these are questions for the implementation. Here it should be a clear hardcoded expected value (well using QDir::homePath and QUrl::fromLocalFile if necessary, of course).

> kurifilter.cpp:270
>  
>  KUriFilterData::KUriFilterData(const QString &url)
> +    : d(new KUriFilterDataPrivate(QUrl::fromUserInput(url, QLatin1String(""), QUrl::AssumeLocalFile), url))

I would very very much like that KUriFilterData is left untouched if possible. Is there no way do call this in the caller instead? (which would then call the QUrl method)

The whole point of KUriFilterData was to not have any string-url conversion logic itself, but leave that to the actual uri filters.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8920

To: emateli, #frameworks, dfaure
Cc: #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20171201/08fbd38e/attachment.html>