Bug 374913

I just updated my openSUSE tumbleweed system and it is broken there now. So  it is definitely a regression in Kate and not a packaging issue.

new version (tumbleweed snapshot 20170112):
```
Kate Version 16.12.0
KDE Frameworks 5.30.0
Qt 5.7.1 (built against 5.7.1)
The xcb windowing system
```

Old version was a snapshot sometime in December still with 16.08 and Frameworks 5.29 or 5.28

Seems like the problem might be in Qt. I added some debug output and it seems 

Qt is calling KDEPlatformFileDialogHelper::selectFile(const QUrl &filename) by simply concatenating filename with the CWD because it does QUrl::fromLocalFile("myfile.cpp") which results in "CWD/myfile.cpp"

I'll see if I can work around this in plasma-integration

https://code.woboq.org/qt5/qtbase/src/widgets/dialogs/qfiledialog.cpp.html#1048
```
    if (!d->usingWidgets()) {
        QUrl url = QUrl::fromLocalFile(filename);
        if (QFileInfo(filename).isRelative()) {
            QDir dir(d->options->initialDirectory().toLocalFile());
            url = QUrl::fromLocalFile(dir.absoluteFilePath(filename));
        }
        d->selectFile_sys(url);
        d->options->setInitiallySelectedFiles(QList<QUrl>() << url);
        return;
    }
```

I have submitted https://codereview.qt-project.org/#/c/182661/ and https://phabricator.kde.org/D4193

*** Bug 380024 has been marked as a duplicate of this bug. ***

*** Bug 380104 has been marked as a duplicate of this bug. ***

Bug fix https://codereview.qt-project.org/#/c/182661/2 is still unreviewed because of coding style issues. Because it is serious issue for KDE, please resubmit it with required changes. Thanks.

*** Bug 374296 has been marked as a duplicate of this bug. ***

(In reply to pyrkosz from comment #6)
> Bug fix https://codereview.qt-project.org/#/c/182661/2 is still unreviewed
> because of coding style issues. Because it is serious issue for KDE, please
> resubmit it with required changes. Thanks.

Patch3 seems to be style-consistent .. i see no reason to refuse it

same problem here ;( still no solution?

(In reply to Alexander Zhigalin from comment #7)
> *** Bug 374296 has been marked as a duplicate of this bug. ***

please submit a vote :) https://bugs.kde.org/page.cgi?id=voting/user.html&bug_id=374913#vote_374913

*** Bug 384311 has been marked as a duplicate of this bug. ***

Is there other patches needed to than the one mentioned here 
https://codereview.qt-project.org/#/c/182661/2
to get the desired behavoir? 

Tested with qt5.9.1 and the above patch and still see the main issue at hand 

Test setup:
- distro - kde-neon - fully updated
- Qt - custom built qt-5.9.1 directly from git - with the above patch applied
- Kate - from kde-neon

Test: 
a) start new kate session
b) open file via sftp or fish 
c) once open - immediately try file->open 

Result: 
- you get the file-open dialog but instead of the sftp/fish'ed location; 
  the location displayed is the current local direct from where I launched kate
- the "Name" field of the dialog populated with the filename opened from step b)

Expected results:
- the remote directory location should be displayed

(In reply to Darren Lissimore from comment #12) 
> Tested with qt5.9.1 and the above patch and still see the main issue at hand 

does that mean?: you patched a 5.9.1 with "patch set 3" and still experience that issue

(In reply to Christoph from comment #13)
> (In reply to Darren Lissimore from comment #12) 
> > Tested with qt5.9.1 and the above patch and still see the main issue at hand 
> 
> does that mean?: you patched a 5.9.1 with "patch set 3" and still experience
> that issue

Yes - with patch-set 3.  

I added in a qDebug() before the patch directory.isLocalFile() check in the patch just verify that the patch is being hit. 
I see the qDebug() output -- yet for a fish'ed file or sftp'ed file I still get as I described above.

Are any others having success with patch-set 3?
- I could be missing something - but it doesn't look like it.

*** Bug 383963 has been marked as a duplicate of this bug. ***

*** Bug 353214 has been marked as a duplicate of this bug. ***

that would imply that Alex's patch doesnt work out and a new approach must be found :-o :-(

I found that i needed both the Qt patch and the KDE patch for correct behavior.

(In reply to Alex Hermann from comment #18)
> I found that i needed both the Qt patch and the KDE patch for correct
> behavior.

did you commit the patches back ?

*** Bug 372208 has been marked as a duplicate of this bug. ***

*** Bug 391869 has been marked as a duplicate of this bug. ***

Git commit bfd41a95530f90ee8d44cbcfd1fa8c62978334a2 by Alex Richardson.
Committed on 08/04/2018 at 10:06.
Pushed by arichardson into branch 'master'.

KDEPlatformFileDialog: Fix initial directory selection for remote files

Summary:
Previously KDEPlatformFileDialogHelper::selectFile() would change
options()->initialDirectory() unconditionally even if it was already
set by the QFileDialog code. Since Qt 5.7.1 it is no longer necessary
to derive initialDirectory from the selectFile() call. In fact it is
actuall harmful since it will now override the correct initial directory
that was set by Qt. Without this patch I got the following debug output:

```
KDEPlatformFileDialogHelper::setDirectory QUrl("sftp://server/home/alr48/cheri/build_sdk.sh")
KDEPlatformFileDialogHelper::setDirectory QUrl("sftp://server/home/alr48/cheri/build_sdk.sh")
KDEPlatformFileDialogHelper::selectFile QUrl("file:///home/alex/build_sdk.sh")
KDEPlatformFileDialogHelper::setDirectory QUrl("file:///home/alex/)
```
The final setDirectory() call is actually a call to
`setDirectory(options->initialDirectory())` which was set in `selectFile()`.

We now depend on Qt 5.9 so we can remove this code without a check for
version >= 5.7.1.

Test Plan: Remote directory is now opened correctly (tested with Qt 5.10.0)

Reviewers: #plasma, elvisangelaccio

Reviewed By: elvisangelaccio

Subscribers: ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D4193

M  +0    -5    src/platformtheme/kdeplatformfiledialoghelper.cpp

https://commits.kde.org/plasma-integration/bfd41a95530f90ee8d44cbcfd1fa8c62978334a2

Git commit cc064e81c6ed51f3b4422c2f2347e5f4e090e628 by Alex Richardson.
Committed on 08/04/2018 at 14:05.
Pushed by arichardson into branch 'Plasma/5.12'.

KDEPlatformFileDialog: Fix initial directory selection for remote files

Summary:
Previously KDEPlatformFileDialogHelper::selectFile() would change
options()->initialDirectory() unconditionally even if it was already
set by the QFileDialog code. Since Qt 5.7.1 it is no longer necessary
to derive initialDirectory from the selectFile() call. In fact it is
actuall harmful since it will now override the correct initial directory
that was set by Qt. Without this patch I got the following debug output:

```
KDEPlatformFileDialogHelper::setDirectory QUrl("sftp://server/home/alr48/cheri/build_sdk.sh")
KDEPlatformFileDialogHelper::setDirectory QUrl("sftp://server/home/alr48/cheri/build_sdk.sh")
KDEPlatformFileDialogHelper::selectFile QUrl("file:///home/alex/build_sdk.sh")
KDEPlatformFileDialogHelper::setDirectory QUrl("file:///home/alex/)
```
The final setDirectory() call is actually a call to
`setDirectory(options->initialDirectory())` which was set in `selectFile()`.

We now depend on Qt 5.9 so we can remove this code without a check for
version >= 5.7.1.

Test Plan: Remote directory is now opened correctly (tested with Qt 5.10.0)

Reviewers: #plasma, elvisangelaccio

Reviewed By: elvisangelaccio

Subscribers: ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D4193

M  +0    -5    src/platformtheme/kdeplatformfiledialoghelper.cpp

https://commits.kde.org/plasma-integration/cc064e81c6ed51f3b4422c2f2347e5f4e090e628

loving it :-) i hope it will get ported to KDE Frameworks 5.44.0 / Qt 5.9.4

(In reply to Alex Richardson from comment #23)
> Git commit cc064e81c6ed51f3b4422c2f2347e5f4e090e628 by Alex Richardson.
> Committed on 08/04/2018 at 14:05.
> Pushed by arichardson into branch 'Plasma/5.12'.

works flawlessly with 5.11.5 too here at a stable Gentoo Linux and fixes a painful issue I had here with KDE - thx.

I believe this fix was wrong, it broke the testSelectUrl unittest, i.e. the case where the application would call selectUrl directly.

I have just submitted what I believe to be the right fix for this bug, in Qt:
https://codereview.qt-project.org/235473

Sorry about that, I didn't test that case.

thank you !

Unfortunately after having been fixed a while, this bug has returned in the latest round of updates to openSuSE 15.0.

Opening from a file on SFTP drops you back in at your local home directory instead of browsing the server.

I'm using all the most up-to-date opeSuSE versions of relevant packages, as of today. I can't say if this is a openSuSE issue or a Plasma issue in this case. 

Anyone else got the same issue?

(In reply to H Richardson from comment #29)
> Unfortunately after having been fixed a while, this bug has returned in the
> latest round of updates to openSuSE 15.0.

means you are using dolphin 17.12.3 now ?

(In reply to H Richardson from comment #29)
> Unfortunately after having been fixed a while, this bug has returned in the
> latest round of updates to openSuSE 15.0.
> 
> Opening from a file on SFTP drops you back in at your local home directory
> instead of browsing the server.
> 
> I'm using all the most up-to-date opeSuSE versions of relevant packages, as
> of today. I can't say if this is a openSuSE issue or a Plasma issue in this
> case. 
> 
> Anyone else got the same issue?

i am indeed deeply shocked :-o @Alex did they drop your commit ?

Working again for me in KDE 5.53.0. I had to upgrade OpenSUSE 15 KDE to separate KDE repository as vanilla OpenSUSE + updates still had the bug.

(In reply to H Richardson from comment #32)
> Working again for me in KDE 5.53.0. I had to upgrade OpenSUSE 15 KDE to
> separate KDE repository as vanilla OpenSUSE + updates still had the bug.

can you please point out which (of your) repos exactly serve the fixed versions 

:) kindly -c-

I did some investigation today, and this does work in latest Plasma 5.12 LTS (with Qt 5.9.7) and also with all latest upstream versions (Plasma 5.15.3, Qt 5.12.2).

Closing as FIXED again therefore.

It got indeed broken in Leap 15.0 by a plasma-integration update that includes https://cgit.kde.org/plasma-integration.git/commit/?h=Plasma/5.12&id=4848bec177b2527662098f97aa745bb839bc15e3. The reason is that 15.0 ships with Qt 5.9.4 though and doesn't have this fix yet that's mentioned in comment#3:
https://code.qt.io/cgit/qt/qtbase.git/commit/?h=5.9&id=4cd90a3579986ae7441c3e982a5f34cdfd92c152

That's tracked downstream now though:
http://bugzilla.opensuse.org/show_bug.cgi?id=1129662