[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: D4193: KDEPlatformFileDialog: Fix initial directory selection for remote files
From: Alex Richardson <noreply () phabricator ! kde ! org>
Date: 2018-04-04 11:03:05
Message-ID: 20180404110305.1.414433A6AEAB12A1 () phabricator ! kde ! org
[Download RAW message or body]
arichardson updated this revision to Diff 31284.
arichardson edited the summary of this revision.
arichardson added a comment.
- Removed version check since we now depend on Qt 5.9
- Updated commit message
REPOSITORY
R135 Integration for Qt applications in Plasma
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D4193?vs=21216&id=31284
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D4193
AFFECTED FILES
src/platformtheme/kdeplatformfiledialoghelper.cpp
To: arichardson, #plasma, elvisangelaccio
Cc: ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, plasma-devel, \
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, \
apol, mart
[Attachment #3 (unknown)]
<table><tr><td style="">arichardson updated this revision to Diff 31284.<br \
/>arichardson edited the summary of this revision. <a \
href="https://phabricator.kde.org/transactions/detail/PHID-XACT-DREV-zxnnrxqwze7fjf4/">(Show \
Details)</a><br />arichardson added a comment. </td><a style="text-decoration: none; \
padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; \
border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to \
bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" \
href="https://phabricator.kde.org/D4193">View Revision</a></tr></table><br \
/><div><div><ul class="remarkup-list"> <li class="remarkup-list-item">Removed version \
check since we now depend on Qt 5.9</li> <li class="remarkup-list-item">Updated \
commit message</li> </ul></div></div><br /><div><strong>CHANGES TO REVISION \
SUMMARY</strong><div><div style="white-space: pre-wrap; color: #74777D;"><span \
style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">We \
currently get the following sequence of calls:<br /> <br />
KDEPlatformFileDialogHelper::setDirectory \
QUrl("sftp://server/home/alr48/cheri/build_sdk.sh")</span><span \
style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, \
.6);">Previously KDEPlatformFileDialogHelper::selectFile() would change<br /> \
options()->initialDirectory() unconditionally even if it was already<br /> set by \
the QFileDialog code. Since Qt 5.7.1 it is no longer necessary</span><br /> <span \
style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, \
.7);">KDEPlatformFileDialogHelper::set</span><span style="padding: 0 2px; color: \
#333333; background: rgba(151, 234, 151, .6);">to derive initial</span>Directory \
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, \
.7);">QUrl("sftp://server/home/alr48/cheri/build_sdk.sh")</span><span \
style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">from the \
selectFile() call. In fact it is</span><br /> <span style="padding: 0 2px; color: \
#333333; background: rgba(251, 175, 175, \
.7);">KDEPlatformFileDialogHelper::selectFile \
QUrl("file:///home/alex/build_sdk.sh")</span><span style="padding: 0 2px; \
color: #333333; background: rgba(151, 234, 151, .6);">actuall harmful since it will \
now override the correct initial directory</span><br /> <span style="padding: 0 2px; \
color: #333333; background: rgba(251, 175, 175, \
.7);">KDEPlatformFileDialogHelper::setDirectory \
QUrl("file:///home/alex/)</span><span style="padding: 0 2px; color: #333333; \
background: rgba(151, 234, 151, .6);">that was set by Qt. Without this patch I got \
the following debug output:</span><br /> <br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, \
.7);">Previously </span><span style="padding: 0 2px; color: #333333; background: \
rgba(151, 234, 151, .6);">```<br /> </span>KDEPlatformFileDialogHelper::se<span \
style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, \
.7);">lectFile() would change</span><span style="padding: 0 2px; color: #333333; \
background: rgba(151, 234, 151, .6);">tDirectory \
QUrl("sftp://server/home/alr48/cheri/build_sdk.sh")</span><br /> <span \
style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, \
.7);">options()->initial</span><span style="padding: 0 2px; color: #333333; \
background: rgba(151, 234, 151, \
.6);">KDEPlatformFileDialogHelper::set</span>Directory<span style="padding: 0 2px; \
color: #333333; background: rgba(251, 175, 175, .7);">() unconditionally even if it \
was already</span><span style="padding: 0 2px; color: #333333; background: rgba(151, \
234, 151, .6);"> QUrl("sftp://server/home/alr48/cheri/build_sdk.sh")</span><br \
/> <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, \
.7);">set by the Q</span><span style="padding: 0 2px; color: #333333; background: \
rgba(151, 234, 151, .6);">KDEPlatform</span>FileDialog<span style="padding: 0 2px; \
color: #333333; background: rgba(251, 175, 175, .7);"> code. The final setDirectory() \
call is actually a call</span><span style="padding: 0 2px; color: #333333; \
background: rgba(151, 234, 151, .6);">Helper::selectFile \
QUrl("file:///home/alex/build_sdk.sh")</span><br /> <span style="padding: 0 \
2px; color: #333333; background: rgba(251, 175, 175, .7);">to \
setDirectory(options->initial</span><span style="padding: 0 2px; color: #333333; \
background: rgba(151, 234, 151, \
.6);">KDEPlatformFileDialogHelper::set</span>Directory<span style="padding: 0 2px; \
color: #333333; background: rgba(251, 175, 175, .7);">()) which was set in the \
selectFile(</span><span style="padding: 0 2px; color: #333333; background: rgba(151, \
234, 151, .6);"> QUrl("file:///home/alex/</span>)<br /> <span style="padding: 0 \
2px; color: #333333; background: rgba(251, 175, 175, .7);">call. It no longer seems \
to be required to derive initialDirectory from the</span><span style="padding: 0 2px; \
color: #333333; background: rgba(151, 234, 151, .6);">```</span><br /> <span \
style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, \
.7);">selectFile() call and this will now to override the correct initial \
directory</span><span style="padding: 0 2px; color: #333333; background: rgba(151, \
234, 151, .6);">The final setDirectory() call is actually a call to</span><br /> \
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, \
.7);">that was set by Qt.</span><span style="padding: 0 2px; color: #333333; \
background: rgba(151, 234, 151, .6);">`setDirectory(options->initialDirectory())` \
which was set in `selectFile()`.<br /> <br />
To completely fix selection of remote URLs this also depends on</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">Qt \
should not be passing a local URL when the actual directory URL is remote</span><span \
style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, \
.6);">https://codereview.qt-project.org/#/c/182661/ which is now in the 5.9</span><br \
/> b<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, \
.7);">ut the code in QFileDialogPrivate::init() unconditionally sets a local \
URL</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, \
151, .6);">ranch of Qt and should be part of the next 5.9 point release and 5.11.<br \
/> <br />
We now depend on Qt 5.9 so we can remove this code without a check for</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, \
.7);">until https://codereview.qt-project.org/#/c/182661/ or another fix is \
submitted</span><span style="padding: 0 2px; color: #333333; background: rgba(151, \
234, 151, .6);">version >= 5.7.1</span>.<div style="padding: 8px \
0;">...</div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R135 \
Integration for Qt applications in Plasma</div></div></div><br /><div><strong>CHANGES \
SINCE LAST UPDATE</strong><div><a \
href="https://phabricator.kde.org/D4193?vs=21216&id=31284">https://phabricator.kde.org/D4193?vs=21216&id=31284</a></div></div><br \
/><div><strong>BRANCH</strong><div><div>master</div></div></div><br \
/><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D4193">https://phabricator.kde.org/D4193</a></div></div><br \
/><div><strong>AFFECTED \
FILES</strong><div><div>src/platformtheme/kdeplatformfiledialoghelper.cpp</div></div></div><br \
/><div><strong>To: </strong>arichardson, Plasma, elvisangelaccio<br /><strong>Cc: \
</strong>ngraham, krzyc, anthonyfieroni, elvisangelaccio, graesslin, plasma-devel, \
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, \
apol, mart<br /></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic