KUrlRequester: Show a popup to select between File and Directory before displaying the file dialog.
Details
Details
- Reviewers
ngraham dfaure - Group Reviewers
Frameworks - Commits
- R241:95d0b44a3837: Fix desktop link to file or directory
Right click onto desktop, select 'Create new' and then
'Basic link to file or directory' or 'Link to Location (URL)'.
Click the file selection icon. A popup is displayed.
If 'File' is selected, a file dialog is displayed, showing files and directories, allowing the selection of a file.
If 'Directory' is selected, a file dialog is displayed, showing only directories, allowing the selection of a directory.
Without the patch, no popup is displayed, only directories are shown in the file dialog and only a directory can be selected.
BUG: 357171
FIXED-IN: 5.59
Diff Detail
Diff Detail
- Repository
- R241 KIO
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
This doesn't work. Try creating a symlink to a directory. In the file picker dialog, the Open button doesn't work.
See https://bugs.kde.org/show_bug.cgi?id=357171 for more context.
Comment Actions
Interesting solution. However the directory use case still doesn't work. If I use the directory chooser to choose /home/nate/Pictures, the path fiels says /home/nate/ and the file name field says file:///home/nate/
Comment Actions
Hmm, that's confusing. I would recommend using the decided directory chooser rather than rolling your own.
Comment Actions
Is there a different decided directory chooser than QFileDialog?
It's the way QFileDialog handles directories. If you click a directory, it not added to the selection, other than a file, which is. So, if QFileDialog::selectedUrls() is called, nothing is in the selection and the current path is returned.
Comment Actions
This is the right way to show a directory chooser dialog: https://cgit.kde.org/plasma-workspace.git/tree/wallpapers/image/image.cpp#n356
Comment Actions
Thanks, I've learned that QFileDialog only creates a well behaving directory chooser, if the QFileDialog::ShowDirsOnly option is set to true _after_ having set the fileMode to QFileDialog::Directory. If you do it the other way around, it doesn't add directories to the selection.
Now it's necessary to delete an already created file dialog and make a new one, if you change your mind and choose the file chooser after having selected a directory chooser, or vice versa. Otherwise it would crash.
Comment Actions
Thanks, this works well now. The UI and behavior seem sane, and I've just got a few code changes requested:
| src/widgets/kurlrequester.cpp | ||
|---|---|---|
| 470 | This should probably be folder-new for visual consistency with the file icon you've chosen | |
| 474 | This feels overly clever and fragile. The more typical approach would be to connect the actions' triggered signals to lambda functions that execute the code that you currently have in the if/else blocks below. | |
| src/widgets/kurlrequester.cpp | ||
|---|---|---|
| 291 | Ok, renamed. This variable is there to remember that the mode initially was set to 'KFile::File | KFile::Directory'. Without, you couldn't change your mind and select 'Directory' after selecting 'File' or vice versa. | |
Comment Actions
Ah! So "DirOrFile" means the user can see and choose both directories and files? Maybe call this ModeWasDirAndFile. I kept reading this was "mode was dirs or mode was files" (which made me say "what else is there?"), while now I think I understand it means "mode was (both dir+files)", right?
Won't this additional popup be annoying for other applications using KUrlRequester? I'm afraid that one use case where it makes sense, pollutes other use cases.
Can you confirm it only shows up when saving, and when the mode is "dir+files"?
| src/widgets/kurlrequester.cpp | ||
|---|---|---|
| 255 | I'd call this createFileDialog() | |
Comment Actions
Yes. In KDE4, with KFileDialog, this used to work, KFileDialog can select both dirs and files, but QFileDialog can't.
Won't this additional popup be annoying for other applications using KUrlRequester? I'm afraid that one use case where it makes sense, pollutes other use cases.
Can you confirm it only shows up when saving, and when the mode is "dir+files"?
This popup only shows up if
((fileDialogMode & KFile::Directory) && (fileDialogMode & KFile::File))
or ModeWasDirAndFile, which can only be set to true if mode previously was
((fileDialogMode & KFile::Directory) && (fileDialogMode & KFile::File))
In this case there always needs to be a decision between File and Directory before the QFileDialog is created, because QFileDialog cannot be set up to be a chooser for both at the same time.
All use cases which want to get an url from KUrlRequester and set the mode to both KFile::Directory and KFile::File must get the user's decision before the QFileDialog is shown.
AcceptMode is independent from this, it can be both AcceptOpen and AcceptSave.
Comment Actions
OK. So a likely fallback of this patch is that some applications which are (possibly erroneously) setting Dir+File as mode (in order to select just a file, for instance), will now bother the user with a popup. If that happens, the applications will need to be fixed to pass just File.
I just tagged 5.57.0-rc1, so this can go in, it will give us one month to test it.
Comment Actions
As a side note (not directly related to the change being discussed here): I think this is a bug in kio/plasma-integration, since the way this needs to be done with the native Plasma file dialog is inconsistent with what the non-native Qt dialog does, s. https://bugs.kde.org/show_bug.cgi?id=406464 .
Comment Actions
Because it has new strings though, I'm afraid this is 5.60 material because we're past the 5.59 string freeze. Making a note on my calendar so it doesn't get lost again...
Comment Actions
I think this can go in now, as the i18n strings are not actually new - grep src/widgets/fileundomanager.cpp for i18n("File") and i18n("Directory").