DevHeads.net

Review Request 125476: Fix QUrl usage when calling QFileDialog::getExistingDirectory()

Review request for kdelibs and David Faure.

Repository: kio

Description
- Passing openUrl.toString() is wrong, it should have been openUrl.path(), since the former has a scheme, and QFileDialog will call QUrl::fromLocalFile() on it.
- QFileDialog::getExistingDirectory() internally calls QFileDialog::getExistingDirectoryUrl() and converts the result to a local file string, which we then were re-converting to QUrl again, so instead just call getExistingDirectoryUrl() directly.

Diffs
src/widgets/kurlrequester.cpp 1b3bbdf

Diff: <a href="https://git.reviewboard.kde.org/r/125476/diff/" title="https://git.reviewboard.kde.org/r/125476/diff/">https://git.reviewboard.kde.org/r/125476/diff/</a>

Testing

Thanks,

Sergio Martins

Comments

Re: Review Request 125476: Fix QUrl usage when calling QFileDial

By Sergio Martins at 10/02/2015 - 10:19

(Updated Oct. 2, 2015, 3:19 p.m.)

Status
This change has been marked as submitted.

Review request for kdelibs and David Faure.

Changes
Submitted with commit e46dc22d5fe84ed0a64e0712d8083756f99b3fa5 by Sergio Martins to branch master.

Repository: kio

Description
- Passing openUrl.toString() is wrong, it should have been openUrl.path(), since the former has a scheme, and QFileDialog will call QUrl::fromLocalFile() on it.
- QFileDialog::getExistingDirectory() internally calls QFileDialog::getExistingDirectoryUrl() and converts the result to a local file string, which we then were re-converting to QUrl again, so instead just call getExistingDirectoryUrl() directly.

Diffs
src/widgets/kurlrequester.cpp 1b3bbdf

Diff: <a href="https://git.reviewboard.kde.org/r/125476/diff/" title="https://git.reviewboard.kde.org/r/125476/diff/">https://git.reviewboard.kde.org/r/125476/diff/</a>

Testing

Thanks,

Sergio Martins

Re: Review Request 125476: Fix QUrl usage when calling QFileDial

By David Faure at 10/02/2015 - 02:32

Ship it!

Looks good, except for a bug in the commit log. You meant "should have been toLocalFile()" instead of "should have been path()" ;)

- David Faure

On Oct. 1, 2015, 8:58 p.m., Sergio Martins wrote:

Re: Review Request 125476: Fix QUrl usage when calling QFileDial

By David Faure at 10/02/2015 - 11:38

No it doesn't, not on Windows.

- David

On Oct. 2, 2015, 3:19 p.m., Sergio Martins wrote:

Re: Review Request 125476: Fix QUrl usage when calling QFileDial

By Sergio Martins at 10/02/2015 - 10:16

which return the same thing for local files, innit ?

- Sergio

On Oct. 1, 2015, 8:58 p.m., Sergio Martins wrote:

Re: Review Request 125476: Fix QUrl usage when calling QFileDial

By Aleix Pol at 10/01/2015 - 17:54

Ship it!

Looks good to me.

- Aleix Pol Gonzalez

On Oct. 1, 2015, 10:58 p.m., Sergio Martins wrote: