[desktop:/ KIO] Strip superfluous slashes and fixup local root url
AbandonedPublic

Authored by broulik on Mar 3 2017, 3:24 PM.

Details

Reviewers
hein
dfaure
Group Reviewers
Plasma
Summary

When dragging a file from one desktop:/ to another desktop:/ KIO wouldn't notice it's the same location as the source would be
file:///home/foo/Desktop//bar
whereas the destination is
file:///home/foo/Desktop/./bar
This patch strips those unneccessary intermediate path segments.

Test Plan

I was surprised that KIO/KFileItem/UDSEntry wasn't smarter here and this patch is probably wrong on many levels.. :)

Diff Detail

Repository
R120 Plasma Workspace
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik created this revision.Mar 3 2017, 3:24 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 3 2017, 3:24 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik edited the summary of this revision. (Show Details)Mar 3 2017, 3:24 PM
dfaure edited edge metadata.Mar 3 2017, 3:30 PM

QDir::cleanPath or QUrl::NormalizePathSegments can help making this fix more generic.

But I'm curious, where does the "/." come from in the first place?

I don't know. The rewritten url for desktop:/ is clean (/home/foo/Desktop) but in KFileItem I see tons of places that are like path + '/' + name but I haven't figured out where the "." actuall comes from...

It's look like relative path to me. If i'm not wrong KFileItem::setLocalPath should resolve relative path ?

broulik added a comment.EditedMar 6 2017, 5:20 PM

For some reason the UDS_LOCAL_PATH isn't properly filled, KFileItem localPath() is still desktop:/ but I got lost in KIO code trying to figure it out :(

This also results in bad side-effects like the "Move To Trash" option not being available, just "Delete"

dfaure added a comment.Mar 7 2017, 8:50 AM

KFileItem::localPath() should not ever ever be a URL like desktop:/. That would be very wrong.
It just returns what you set in UDS_LOCAL_PATH, so it's kio_desktop you should debug, not kio ;)
Well except for KIO::ForwardingSlaveBase, called by kio_desktop -- note that ForwardingSlaveBase::prepareUDSEntry already sets UDS_LOCAL_PATH, but ok you call insert again which replaces that value.

The code looks ok to me, are you 100% sure about your diagnostic that localPath() returns desktop:/... ?

Taking for example the context menu code in FolderView:
It does KFileItem = m_dirModel->itemForIndex(index);

item.localPath(): ""
item.isLocalFile(): false
item.mostLocalUrl(): QUrl("desktop:/foo")
item.targetUrl(): QUrl("desktop:/foo")
item.entry().stringValue(KIO::UDSEntry::UDS_LOCAL_PATH): ""

If you right click an item in desktop:/ the "Move to Trash" option will not be available, both in FolderView and Dolphin because it doesn't consider it a local file.

broulik abandoned this revision.Mar 27 2017, 5:02 PM

Somehow I must have screwed up my setup somehow, I can't reproduce the issues I had anymore and never had it on my laptop to begin with.