Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add root_subfolder to FileDialog #59089

Merged
merged 1 commit into from Jul 1, 2022
Merged

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Mar 13, 2022

Closes godotengine/godot-proposals#3929
root_subfolder will make the specified sub-folder into a "fake root" of the FileDialog. You won't be able to go to parent directory and the path display changes.

Without root_subfolder:
godot windows tools 64_vIhPXogvZm

root_subfolder set to Test:
godot windows tools 64_iwfuPgRhJF

@KoBeWi KoBeWi added enhancement feature proposal cherrypick:3.x Considered for cherry-picking into a future 3.x release topic:gui labels Mar 13, 2022
@KoBeWi KoBeWi added this to the 4.0 milestone Mar 13, 2022
@KoBeWi KoBeWi requested review from a team as code owners March 13, 2022 00:41
@KoBeWi KoBeWi force-pushed the I_am_root branch 2 times, most recently from a39d717 to 10cbdcc Compare March 13, 2022 00:45
@timothyqiu
Copy link
Member

I can still go up by typing .. in the Path box, or select a parent directory file directly by typing ../filename in the File box 😛

Kind of the same issue as #34194. Although that is about the real root, and this one is about the fake root.

@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 13, 2022

I can still go up by typing .. in the Path box

How? I tested that, you can even see on the GIF it's not possible.

select a parent directory file directly by typing ../filename in the File box 😛

Meh, if you don't know filename, then you can't select it. So blocking folder access is enough.

@timothyqiu
Copy link
Member

timothyqiu commented Mar 13, 2022

How? I tested that, you can even see on the GIF it's not possible.

I don't know why, but .. works for me...

Peek 2022-03-13 20-12

Meh, if you don't know filename, then you can't select it. So blocking folder access is enough.

That makes sense for "Open". But the restriction leaks for "Save". The user don't have to know about .., mistyping the filename with a leading / breaks it...

p.s. By "break", I mean when the dialog is "confirmed", the user should be able to assume current_filename not pointing to anything outside root_subfolder.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved on the principle in PR review meeting, but the issues highlighted by @timothyqiu should likely still be fixed before merging.

@KoBeWi
Copy link
Member Author

KoBeWi commented Jun 28, 2022

Rebased. Still can't reproduce the .. bug. As for files, FileDialog seems to have some general problem with picking files. You can e.g. pick a folder when it expects filename and it even emits file_selected signal. The issue existed even before this PR.

@akien-mga
Copy link
Member

Let's merge and track remaining/preexisting issues in new bug reports.

@akien-mga akien-mga merged commit daec5be into godotengine:master Jul 1, 2022
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the I_am_root branch July 1, 2022 10:01
@KoBeWi KoBeWi removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow defining a folder as the root folder in FileDialog to prevent users from accessing outside files
4 participants