Fix display of remote:/ in the qfiledialog
ClosedPublic

Authored by apol on Nov 3 2017, 4:54 PM.

Details

Summary

remote: is designed to redirect every entry so it has an entry that
initially is defined as "remote:/network" then it changes to "network:/"
through setUrl which currently is passing QUrl().fileName() which is empty
for network:, smb: or mtp:.
This patch changes it so at least the displayed text isn't overriden if it
was coming from the UDS_DISPLAY_NAME entry in the UDS tuple we received.

Test Plan

This behavior can be tested and confirmed using the qfiledialogtest in
plasma-integration/tests.

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.
apol created this revision.Nov 3 2017, 4:54 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 3 2017, 4:54 PM
davidedmundson accepted this revision.Nov 3 2017, 7:37 PM
This revision is now accepted and ready to land.Nov 3 2017, 7:37 PM
ngraham added a subscriber: ngraham.Nov 3 2017, 9:20 PM
This revision was automatically updated to reflect the committed changes.
dfaure added a subscriber: dfaure.Nov 5 2017, 8:30 PM
dfaure added inline comments.
src/core/kfileitem.cpp
559

Wouldn't it be safer to say if (!name.isEmpty()), for the actual bug you were experiencing?

Otherwise I'm afraid that this commit breaks a number of things, like after renaming a file, the new name doesn't appear because the old "display name" still applies, or the documented use case for setName which is kfind wanting to display subdir/subsubdir/file.txt (same issue, UDS_DISPLAY_NAME would prevent that from working).

apol added inline comments.Nov 6 2017, 11:37 AM
src/core/kfileitem.cpp
559

My thinking was that if the display name was set through the UDSEntry, one would want it to continue coming from a UDSEntry. e.g. if file:///boring.txt display name is Magnificient Text File and we change its url into file:///boring2.txt we would still want the same. Or we'd issue a new UDSEntry.

I'm happy to change it to

-    if (!d->m_entry.contains(KIO::UDSEntry::UDS_DISPLAY_NAME)) {
+    if (!d->m_strName.isEmpty()) {

It was my first approach in fact, it worked fine.

apol added inline comments.Nov 6 2017, 4:29 PM