Bug 32163

Summary: Provide a choose-file signal to allow implementing a custom file chooser
Product: WebKit Reporter: Christian Dywan <christian@twotoasts.de>
Component: WebKit GtkAssignee: Christian Dywan <christian@twotoasts.de>
Status: RESOLVED WONTFIX    
Severity: Enhancement CC: gns@gnome.org, lucian.branescu@gmail.com, tomeu.vizoso@collabora.co.uk, webkit.review.bot@gmail.com
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Implement WebKitFileChooser and choose-file
none
Implement WebKitFileChooser and choose-file, #2
none
Implement WebKitFileChooser and choose-file, #3 abarth: review-

Description From 2009-12-04 10:01:11 PST
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.
------- Comment #1 From 2009-12-17 11:00:22 PST -------
Created an attachment (id=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.
------- Comment #2 From 2009-12-17 11:27:11 PST -------
Created an attachment (id=45096) [details]
Implement WebKitFileChooser and choose-file, #2

Updated, to include the new files.
------- Comment #3 From 2009-12-20 05:48:33 PST -------
(From update of attachment 45096 [details])
Note to myself: I need to fix the documentation header, and add g_return*if_fail to public functions.
------- Comment #4 From 2010-06-25 08:25:03 PST -------
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.
------- Comment #5 From 2010-06-25 08:47:05 PST -------
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.
------- Comment #6 From 2011-02-11 06:16:56 PST -------
Created an attachment (id=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.
------- Comment #7 From 2011-02-11 06:20:05 PST -------
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.
------- Comment #8 From 2011-05-05 01:15:04 PST -------
(From update of attachment 82127 [details])
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.
------- Comment #9 From 2014-03-25 11:10:00 PST -------
The wk1 API is about to be removed, and the wk2 API supports this use-case, so I think this bug is obsolete.