Currently the folderpanel is inert when browsing outside of home because urls end up as 'file:////a/b/c'
Premptive bug fix.
elvisangelaccio |
Dolphin |
Currently the folderpanel is inert when browsing outside of home because urls end up as 'file:////a/b/c'
Premptive bug fix.
Open dolphin
Leave home directory
No Linters Available |
No Unit Test Coverage |
These are in fact the first 5 lines I've ever written in C++. So please be patient if my code is clumsy, ugly or just wrong.
BTW, I did not understand the comment following my lines. "first subdir ... does not end with" ?
src/kitemviews/kfileitemmodel.cpp | ||
---|---|---|
637 ↗ | (On Diff #24592) | QUrl urlToExpand = m_dirLister->url().ajusted(QUrl::StripTrailingSlash); |
Just tested it. It's not working.
url().adjusted(QUrl::StripTrailingSlash) does not strip from 'file:///'
src/kitemviews/kfileitemmodel.cpp | ||
---|---|---|
637 ↗ | (On Diff #24592) | Just tested it. It's not working. |
anthonyfieroni, I was so much involved in figuring out what to do next, I forgot to thank for your comment.
So: Thanks for your comment.
Thanks for the patch. Can you explain exactly what is not working and how to reproduce the bug?
Please check https://phabricator.kde.org/D7477
When navigating outside home
panels/folders/folderspanel.cpp:
332 baseUrl = QUrl::fromLocalFile(QDir::rootPath()); // = "file:///"
this eventually becomes
m_dirLister->url() // = "file:///"
the last slash is not a trailing slash but the root directory, consequently
url().adjusted(QUrl::StripTrailingSlash)
does not remove it (I guess).
651 urlToExpand.setPath(urlToExpand.path() + '/' + subDirs.at(i));
appends a 4th '/' resulting in "file:////<subdir>" which does not exist.
That bug can be reproduced by navigating outside your home folder.
folderspanel does not expand the folder you navigated to.
src/kitemviews/kfileitemmodel.cpp | ||
---|---|---|
637 ↗ | (On Diff #24592) | QUrl::StripTrailingSlash will strip in loop all '/' that why it's not working. Thanks for testing. |
651 ↗ | (On Diff #24592) | You can add check here. QString path = urlToExpand.path(); if (!path.endsWith(QLatin1Char('/')) { path.append(QLatin1Char('/')); } urlToExpand.setPath(path() + subDirs.at(i)); |
src/kitemviews/kfileitemmodel.cpp | ||
---|---|---|
651 ↗ | (On Diff #24592) | At this point is there still a possibility that urlToExpand.path() ends with a slash? Under what conditions? |
src/kitemviews/kfileitemmodel.cpp | ||
---|---|---|
651 ↗ | (On Diff #24592) | It's not possible to ends with '/' at that point but i'm searching for solution without setting empty paths urlToExpand.setPath(path.left(pos - 1)); will set path to empty string. |
src/kitemviews/kfileitemmodel.cpp | ||
---|---|---|
651 ↗ | (On Diff #24592) | What about enforcing a trailing slash in panels/folders/folderspanel.cpp urlToExpand.setPath(urlToExpand.path() + subDirs.at(i) + '/' ? void KFileItemModel::expandParentDirectories(const QUrl &url) is used only once anyway |
@elvisangelaccio, the problem is file:/// + / + subdir result in invalid path. With @dfaure we make a patch in KIO does we should do it here?
Ah right, this is yet another regression from the Qt 5.10 fallout.
That's not exported, we cannot use it from Dolphin.
src/kitemviews/kfileitemmodel.cpp | ||
---|---|---|
651 ↗ | (On Diff #24592) | Here we need to append the slash only if the path() does not end with a slash already. This fixes the bug for me. |
I've incorporated your suggestion and removed most of my code. I tested the code and it works .
But now I'm stuck here https://phabricator.kde.org/differential/diff/24627/
The only option I have on that page is to create a new revsion. But I want to update this one.
Shall I proceed? Or will I mess things up when I do?
I revisited that page. Et voila: now I can update. Which I did.
Strange! Yesterday I could not (maybe it was my firefox).
Do I need to check any of the "Done" checkboxes?
I want to keep going. Is it correct to do
git pull
(work)
arc diff
for the next revision?
For the record:
git pull
(work)
arc diff
did not work.
Propable reasons:
What did work was:
git checkout origin/Applications/17.12
Oh yes, sorry about that. The reason is that this commit has not been merged to the master branch, yet.
So ifyou need to do work on top of it, indeed you need to checkout the stable branch (which currently is Applications/17.12).
As I said I'm a total newbie in this field, so trying to work such things out helps to understand the inner workings of git et al. The downside of being ignorant: mistakes. And apparently I made one. After checking out origin/Applications/17.12
I compiled again and sadly dolphin it did not work as it should. I've fixed that already (in case you're curious: QString path has to be declared outside the loop) but before submitting it I have to sort out the mess of revisions I made here. I'll submit it within the next 3 days.
Side note:
I'm on opensuse Tumbleweed and as a consequence suffering under this bug since 2 weeks or so. On the other hand that made me approach C++ (shiverrrr). And hey, now I'm using my own fixed compiled dolphin.
To wrap things up:
$ git checkout -b otto origin/Applications/17.12 $ git branch master * otto
In new KDevelop session:
import src/CMakeLists.txt build install (to ~/bin/otto/) run in Debugger -> Dolphin works as expected
Then:
$ ~/bin/otto/dolphin Trying to convert empty KLocalizedString to QString. org.kde.dolphin: Ignore KIO url: QUrl("timeline:/lastmonth") org.kde.dolphin: Ignore KIO url: QUrl("timeline:/thismonth") org.kde.dolphin: Ignore KIO url: QUrl("timeline:/yesterday") org.kde.dolphin: Ignore KIO url: QUrl("timeline:/today") org.kde.dolphin: Ignore KIO url: QUrl("search:/images") org.kde.dolphin: Ignore KIO url: QUrl("search:/documents") org.kde.dolphin: Ignore KIO url: QUrl("search:/videos") org.kde.dolphin: Ignore KIO url: QUrl("search:/audio") org.kde.dolphin: Ignore KIO url: QUrl("timeline:/lastmonth") org.kde.dolphin: Ignore KIO url: QUrl("timeline:/thismonth") org.kde.dolphin: Ignore KIO url: QUrl("timeline:/yesterday") org.kde.dolphin: Ignore KIO url: QUrl("timeline:/today") org.kde.dolphin: Ignore KIO url: QUrl("search:/images") org.kde.dolphin: Ignore KIO url: QUrl("search:/documents") org.kde.dolphin: Ignore KIO url: QUrl("search:/videos") org.kde.dolphin: Ignore KIO url: QUrl("search:/audio") qt.accessibility.core: Cannot create accessible child interface for object: PlacesView(0x23ca670) index: 21
-> Dolphin works as expected
Just out of curiosity. Would tell me why you tried to avoid an empty path in
urlToExpand
?