GNOME Bugzilla – Bug 609886
[PATCH] Add the ability to root the File Chooser to a specific URI
Last modified: 2018-04-15 00:04:47 UTC
Created attachment 153743 [details] [review] Patch to add the ability root the file chooser to a specific URI This patch allows the file chooser to limit the user to files and folders within a particular URI, local or remote. The only UI elements (shortcuts, bookmarks, special folders, autocomplete, search results, pathbar buttons, etc.) that will show up are ones within the root URI. This is really useful when needing to restrict the user to a particular local directory (something within their home directory, for instance) or a remote file server (whether ftp, samba, or anything else). It means that a program that needs to request a file on a specific remote server that you're connected to can now reuse GtkFileChooser instead of either having to validate the chosen file after closing the dialog or implementing their own custom browser. When setting a root URI, the current directory will always reflect a directory within that root URI. The user is unable to either browse to a file or type a filename outside of the root URI. The Search entry in the sidebar will only ever display for local files. The Recently Used entry, on the other hand, is always displayed, and will filter the recent files based on the root URI. The root URI will generally be set before the file chooser is displayed. However, as the testfilechooser and testfilechooserbutton apps demonstrate, a new root URI can be set at any time and the UI will update accordingly. This has required some changes to better make the file chooser and related widgets handle reloading and redisplaying of information. As this new root URI concept is similar in ways to the "local only" flag (in that they both restrict what the user can access), the "local only" flag now simply implies a root URI of "file://". It may be worth considering whether to deprecate "local only" in the future in favor of root URIs, but for the time-being, they'll co-exist happily. The documentation has been updated to reflect the new relationship between the two.
Screenshots of this in action: Rooting GtkFileChooser to file:///home/chipx86/Desktop: http://www.flickr.com/photos/chipx86/4355268666/ Demonstrating Recently Used with the Desktop root: http://www.flickr.com/photos/chipx86/4355268718/ Rooting GtkFileChooser to ftp://anonymous@ftp.gnome.org/ http://www.flickr.com/photos/chipx86/4355268638/
> reuse GtkFileChooser instead of either having to validate the chosen file You really need to write a unit test if you promise that validation on the application side won't be needed anymore. And of course the test needs to ensure that normal behaviour doesn't break. Did you consider adding validation to gtk_file_chooser_get_files as well, to enforce your promise? A validation that returns NULL and prints a warning would seem like a good idea.
Thanks for your comments. Just to clarify, when I say that they won't have to do validation, what I mean is that they won't have to work around the fact that the file chooser will let the user pick a file from *anywhere* and then throw up a dialog telling them to try again but to stay in a certain directory this time. With this change, if the root URI is set to sftp://myhost/, then you cannot get to /home or any other place, so there's no need to really perform this check. The testfilechooser and testfilechooserbutton apps demonstrate this if you set something for the Root URI property. I'll look into adding unit tests though, once I can get the existing gtkfilechooser.c tests to even compile. It seems pretty abandoned and broken. Did you want the validation in gtk_file_chooser_get_files itself, or in the interface implementation of it in gtkfilechooserdefault.c?
Wow, this is pretty sweet! Ever since http://www.gnome.org/~federico/docs/file-chooser-extension-spec/#scenario-lockdown there has been an idea about this, but it was never implemented. Do you have a git repository where I can look at smaller patches? Those will probably take me less time/brainpower to review than the big patch you posted. How hard would it be to make it possible to have more than one root, and display the roots in the shortcuts bar? I'm thinking of the case of "thin clients where it's only meaningful to browse your $HOME and a USB stick".
(In reply to comment #3) > Did you want the validation in gtk_file_chooser_get_files itself, or in the > interface implementation of it in gtkfilechooserdefault.c? I think doing that in the function is the only way to ensure that a) the file is inside the root b) possible bugs allowing the user to select a different location don't cause trouble c) make sure that third party file choosers adhere to the restriction.
(In reply to comment #4) > Wow, this is pretty sweet! Ever since > > http://www.gnome.org/~federico/docs/file-chooser-extension-spec/#scenario-lockdown > > there has been an idea about this, but it was never implemented. Glad you like it :) > Do you have a git repository where I can look at smaller patches? Those will > probably take me less time/brainpower to review than the big patch you posted. I have a private one. I could put the patches up. They're a little disorganized, though. If it helps any: http://reviews.rbcommons.com/r/64/diff/ > > How hard would it be to make it possible to have more than one root, and > display the roots in the shortcuts bar? I'm thinking of the case of "thin > clients where it's only meaningful to browse your $HOME and a USB stick". It's a pretty big departure from my change, really.. I'd have to redo most of it. Parts shouldn't be so bad. I could change set_root to add_root (and then have remove_root and clear_roots). The various is_*_in_root functions that are used could then loop through those roots. It feels like it'd make it a lot more complicated to use from an API standpoint, though. "root" wouldn't really make sense anymore either.
Created attachment 156827 [details] [review] Patch to add multi-root support for the file chooser.
I've updated the patch to add the multi-root behavior we discussed. This allows callers to define a list of root URIs that the file chooser is bound to. They can't exit any of the roots, but they can go between them. Each root is displayed in the sidebar (so long as it's not already listed as the home directory, Desktop, an existing volume, or has a parent volume that's already listed as a root). I've also cleaned up my commits a bit and put this up on GitHub, to ease reviewing: http://github.com/chipx86/gtk/compare/master...multiroot-filechooser
Dear ChipX86, You rule. Love, Federico P.S. Please give me a few days to review this; I'm a bit busy ATM but I certainly want to get this functionality into the file chooser!
Hi Federico, I know you're busy lately and I don't want to add to your TODO list too much, but I'm working on a release of VMware Workstation that is going to depend on this functionality and am curious whether there's a chance to get this support reviewed and in for 2.20 or whether it'd be for 2.22 (and when 2.22 would end up being released if slated for that release). Thanks, Christian
I'm reviewing this now. If I have changes to make, I'll push your branch to git.gnome.org and stack my changes on top of it.
I've pushed an updated version of this to the multiroot-filechooser-2-20 branch. I deleted the old multiroot-filechooser branch, which was based somewhere around 2.18, which is too old for me to test now. Happily, the 2-20 version works pretty well on my box so far. Some things need work: - The docstrings say "Since: 2.18" - easy to fix. - If the file chooser already has a current_folder and set_root_uris() is called, it does something different for "cwd is outside of local_only" vs. "cwd is outside the roots". This is a corner case, but I guess these should have the same behavior (and share the same code). - The new API is gtk_file_chooser_set_root_uris(chooser, GSList of char*). The "root-uris" property is of G_TYPE_POINTER. This is ugly from a bindings standpoint - can we do better? Offhand I couldn't remember objects that get passed a list-of-strings... I do want to merge this to master at some point. For the future, it would be nice to lay down a structure for doing lockdown with GSettings, so that the standard file chooser for random apps will automatically be locked down to whatever the sysadmin has configured (maybe $HOME and the corporate shares).
Federico, Since you don't have comments on your blog, what do you mean by GSList of char* is ugly for language bindings? For GI we should be fine as long as that GSList doesn't get changed underneath us or you don't rely on the pointer as an identifier. We just end up recreating it as a python list of strings in pygobject introspection and then back to a GSList when passing it to a Gtk/GLib method. One API where this fails is Groups for GtkOptionButtons. They use the list pointer as an ID for the group a button is in. Since we convert the pointer gets changed when being marshalled and can not be used as a unique identifier in bindings.
Federico, For PyGtk bindings (if you commit this to 2-22 branch), I offer the same response and caveats as johnp. The override for a GSList of char* is easy to deal with.
Why not just use char** / G_TYPE_STRV for the list of uris?
Presumably constructing a persistent strv is a lot more error prone than a linked list. In my opinion though, the list should be an internal implementation detail. There should be an API for adding to, removing from and clearing the roots list. If I am able to send a list of URI's they should be copied into the internal list, not set. Someone could easily append to the list they sent in and the results would be undetermined because the head of the list may be a new pointer.
Thanks for the comments about the list of strings; I'll use a G_TYPE_STRV. Other widgets already use that. The current code just steals the pointer to the GSList; that's obviously wrong. I'll make it use a G_TYPE_STRV that copies the strings internally.
I've just pushed updated branches: multiroot-filechooser-2-20 - the original code for GTK+ 2.20 multiroot-filechooser - a new version rebased on top of today's master; ready for merging into GTK+ 3.0. I'll ask for feedback in gtk-devel-list. (By the way, I've been running multiroot-filechooser-2-20 for a month with zero ill effects. This is very encouraging!)
Awesome! Just saw the post. I'm thrilled that you haven't seen any major regressions :) I'll be honest, I'm a bit surprised that *something* hasn't come up. Sorry I've been unavailable to really hack on the remaining things. I'm a bit swamped with my work here and haven't had the time to even think about it. If anything major comes up that you need me to do, I can try to find some time for it.
Are there plans to force existing application to use this feature? Env. var, gtkfilechooser.ini, etc?
(In reply to comment #20) > Are there plans to force existing application to use this feature? Env. var, > gtkfilechooser.ini, etc? Not yet, sorry (i.e. lockdown is not implemented). But this should be easy to do now that things have been converted to GSettings. If you want to try writing a patch to implement proper lockdown based on the rooting functionality, I'll be glad t oreview it - just create a branch based on the multiroot-filechooser one.
We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.
As announced a while ago, we are migrating to gitlab, and bugs that haven't seen activity in the last year or so will be not be migrated, but closed out in bugzilla. If this bug is still relevant to you, you can open a new issue describing the symptoms and how to reproduce it with gtk 3.22.x or master in gitlab: https://gitlab.gnome.org/GNOME/gtk/issues/new