Bug 32163 – Provide a choose-file signal to allow implementing a custom file chooser

• Component: WebKitGTK
• Status: RESOLVED WONTFIX

[Christian Dywan]

I would like to implement my own file chooser in Midori. For instance to preset the default folder with the last folder used in other file choosers in the application. I'm thinking of "choose-file" with a const gchar* default_filename and returning a gchar*. If the signal is not handled, it should just do the same that is done now.

[Christian Dywan]

Created attachment 45093 [details]
Implement WebKitFileChooser and choose-file

So I implemented a choose-file signal. It passes a WebKitFileChooser object that has a select-multiple property and a uris GSList* property.

I'm not sure whether the object's name is ideal. I couldn't think of a better name. I'm open to suggestions if someone has a different idea.

It seems WebCore can't handle URIs, or at least the internal interface only expects local filenames. So the patch works but will fail if remote files are selected. I'm not sure if the current code would support that.

[Christian Dywan]

Created attachment 45096 [details]
Implement WebKitFileChooser and choose-file, #2

Updated, to include the new files.

[Christian Dywan]

Comment on attachment 45096 [details]
Implement WebKitFileChooser and choose-file, #2

Note to myself: I need to fix the documentation header, and add g_return*if_fail to public functions.

[Lucian]

I'd also need such a signal for Browse, the Sugar browser (http://git.sugarlabs.org/projects/browse).

Afaik, file pickers aren't supposed to target remote URIs anyway. If the feature is needed, you'd have to download the remote URI yourself and pass a local file to the browser, to pass on to the server.

[Lucian]

I've reviewed the patch and thinking from the p.o.v. of Python, the API looks alright.

I hope this will get merged as soon as possible.

[Christian Dywan]

Created attachment 82127 [details]
 Implement WebKitFileChooser and choose-file, #3

Updated patch, added missing return_if_fail and changed uris to filenames because that's what is actually supported and useful.

[WebKit Review Bot]

Attachment 82127 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/gtk/ChangeLog', u'Source/Web..." exit_code: 1

Last 3072 characters of output:
y/parameter_name] [5]
Source/WebKit/gtk/webkit/webkitfilechooser.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.h:72:  The parameter name "file_chooser" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/gtk/webkit/webkitfilechooser.h:73:  Extra space between GSList* and filenames  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitfilechooser.h:76:  The parameter name "file_chooser" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/gtk/webkit/webkitfilechooser.h:76:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:25:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:81:  The parameter name "object" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:83:  The parameter name "object" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:83:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:85:  The parameter name "object" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:85:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:171:  Missing space before ( in switch(  [whitespace/parens] [5]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:198:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:228:  Extra space between gboolean and selectMultiple  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:230:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:247:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:261:  Extra space between GSList* and filenames  [whitespace/declaration] [3]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:263:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:280:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit/gtk/webkit/webkitfilechooser.cpp:295:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 36 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Barth]

Comment on attachment 82127 [details]
 Implement WebKitFileChooser and choose-file, #3

View in context: https://bugs.webkit.org/attachment.cgi?id=82127&action=review

This really needs a proper review by folks knowledgeable in GTK.  Some of the style errors look bogus, but some of them look like they should be addressed.  I'm going to mark this R- for now to remove it from the queue, but please feel encouraged to upload a new version of the paper.  You should probably find one of the gtk folks on IRC and ask them to take a look at your patch.

> Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:602
> +static void chooseFileCb(WebKitWebView* webView, WebKitWebFrame*, WebKitFileChooser* fileChooser)

Cb = ?  I don't understand this abbreviation.

[Gustavo Noronha (kov)]

The wk1 API is about to be removed, and the wk2 API supports this use-case, so I think this bug is obsolete.