dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> anthonyfieroni wrote in Jobs.qml:117-120
> displayDestUrl = destUrl.replace(/^(file:\/{2})/, "")
All of this is wrong. Removing file:/// leaves something that is neither a
correct path nor a correct URL.
You want destUrl.toString(QUrl::PreferLocalFile).
This code being javascript is no excuse -- move all this to C++ code :-)
> Jobs.qml:128
> + if (url[0] === "/") {
> + url = "file://" + url;
> + }
Nooooooo.
This is broken. Testcase: try a filename with a '#' in it.
If the input is indeed a "path or URL" (urgh, that's never good practice, other
than for showing to the user), then the way to turn this into a URL is
QUrl::fromUserInput().
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D19588
To: broulik, #plasma, #vdg, dfaure
Cc: anthonyfieroni, rooty, plasma-devel, jraleigh, GB_2, ragreen, Pitel,
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
