D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

[David Faure]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R135:22da1e661afd: File Dialog: fix testSelectUrl() again, 
i.e. selectUrl() should set the… (authored by dfaure).
Herald added a project: Plasma.

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14440?vs=38681=40018

REVISION DETAIL
  https://phabricator.kde.org/D14440

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp
  src/platformtheme/kdeplatformfiledialoghelper.h

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik
Cc: ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

[David Faure]

dfaure added a comment.


  I'll assume +1 and +1 = +2, and push...

REVISION DETAIL
  https://phabricator.kde.org/D14440

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

[Alex Richardson]

arichardson added a comment.


  +1 LGTM. Thanks!

REVISION DETAIL
  https://phabricator.kde.org/D14440

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

[Anthony Fieroni]

anthonyfieroni added a comment.


  +1 from me.

REVISION DETAIL
  https://phabricator.kde.org/D14440

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

[David Faure]

dfaure updated this revision to Diff 38681.
dfaure added a comment.


  Add a workaround for the current Qt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14440?vs=38653=38681

BRANCH
  Plasma/5.13

REVISION DETAIL
  https://phabricator.kde.org/D14440

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp
  src/platformtheme/kdeplatformfiledialoghelper.h

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

[Alex Richardson]

arichardson added a comment.


  Does this mean for Qt < 5.12 (or .13?) pressing open in kwrite will select 
the cwd again?
  
  I now have almost all my source directories NFS mounted rather than having to 
use sftp so it is no longer such a big issue for me. However, I do wonder if it 
makes sense to keep the current behaviour for older Qt versions? I.e. something 
like `!m_directorySet || QT_VERSION < 5.12`?

REVISION DETAIL
  https://phabricator.kde.org/D14440

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

[David Faure]

dfaure created this revision.
dfaure added reviewers: arichardson, anthonyfieroni, elvisangelaccio, 
plasma-devel, broulik.
dfaure requested review of this revision.

REVISION SUMMARY
  This was fixed initially in commit 7bbbd93 
 
(https://phabricator.kde.org/D3796)
  when the unittest was added, and broken later in commit bfd41a9 

  (https://phabricator.kde.org/D4193, bug 374913) which says Qt takes care
  of taking the initial directory when calling selectUrl(). I cannot see
  that in the Qt code, where selectUrl only calls selectFile_sys.
  
  This breaks bug 374913 again (initial directory for remote files), but the
  right fix for that is https://codereview.qt-project.org/235473

TEST PLAN
  unittest

BRANCH
  Plasma/5.13

REVISION DETAIL
  https://phabricator.kde.org/D14440

AFFECTED FILES
  src/platformtheme/kdeplatformfiledialoghelper.cpp
  src/platformtheme/kdeplatformfiledialoghelper.h

To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik