[webkit-gtk] File chooser signal backported to WebKit1

[Mario Sanchez Prada]

Hi Daniel,

On Tue, 2012-05-22 at 23:32 +0100, Daniel Drake wrote:
> In Sugar, we need to override the file chooser used for file uploads.
> This is now possible with WebKit2 (thanks!)
> https://bugs.webkit.org/show_bug.cgi?id=78491

You're welcome :)

> However OLPC is shipping WebKit1 right now. We'll move to WebKit2 shortly,
> but we need a temporary solution for right now.

I see and I think it makes sense. Please open a bug in bugzilla for this
(in case you haven't already done so).

> So I've backported the signal and API to WebKit1, and tested it.
> I have removed the API around filtering mime types and so on, which wasn't
> implemented anyway in the hardcoded filechooser popup.

Well, the fact that it filtering by MIME type was not implemented in the
current implementation of runOpenPanel in WK1 shouldn't be, IMHO, a
reason not to add it now if we're providing a completely new
implementation, based on signals.

I point this out since I think it's an interesting feature and, given
it's supported in HTML's input element, I think we should add it here
too, as we did for WK2 (see my 2nd patch attached).

> I'm trying to follow the webkit contributing page to post this to bugzilla
> but without success. I can't complete the svn/git checkouts, probably due to
> my slow connection in Nicaragua struggling with the sheer size of WebKit.

I sympathize. WK's repo is so huge that cloning/checking it out is a
pain...

> Would anyone be willing to run the check-webkit-style program for me (if
> appropriate) and prepare-ChangeLog to get this in a format appropriate
> for submission?

I've run check-webkit-style for you and this is the result:

$ check-webkit-style -g HEAD
Source/WebKit/gtk/webkit/webkitfilechooserrequestprivate.h:28:  The
parameter type should use PassRefPtr instead of RefPtr.
[readability/pass_ptr] [5]

Source/WebKit/gtk/webkit/webkitwebview.h:35:  Alphabetical sorting
problem.  [build/include_order] [4]

Source/WebKit/gtk/webkit/webkitwebview.h:170:  Extra space between
WebKitFileChooserRequest and *request  [whitespace/declaration] [3]

Source/WebKit/gtk/webkit/webkitfilechooserrequest.cpp:24:  Alphabetical
sorting problem.  [build/include_order] [4]

Source/WebKit/gtk/webkit/webkitfilechooserrequest.cpp:119:  The
parameter type should use PassRefPtr instead of RefPtr.
[readability/pass_ptr] [5]

Source/WebKit/gtk/webkit/webkitfilechooserrequest.cpp:140:  Tab found;
better to use spaces  [whitespace/tab] [1]
Total errors found: 6 in 11 files


(See the first patch attached to this mail with the needed
 changes to be applied to correct those errors)


About running the prepare-ChangeLog script and filling the details, I'd
say it's too early for that since there are at least some things missing
before this patch can actually get accepted upstream. Off the top of my
head:

 - You're adding new API to WK1 -> you need to add Unit tests.

 - As I said before, I think adding the API for mime types 
   would be a good idea, now you're proposing this. This means
   webkit_file_chooser_request_get_mime_types_filter at least.

 - Also, I'd add the rest of the API present in WK2. Namely:
     webkit_file_chooser_request_get_mime_types
     webkit_file_chooser_request_get_selected_files


About the last two items, see the second patch attached here, where I
already added the needed code (and roughly tested it manually), which
was a fairly immediate task.

However, I'm afraid I can't now deal with the implementation of the unit
test, which will for sure take way more time, so if you could do that
yourself, I'll be happy to re-run the check-webkit-patch script again,
and also the prepareChangeLog one, once you're done with that, if that's
ok to you.

Also, don't forget to file a new bug about this, so we can continue the
discussion and development there more easily.

Thanks!
Mario


[1] https://bugs.webkit.org/show_bug.cgi?id=53600
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-the-few-coding-style-issues-on-Daniel-s-patch.patch
Type: text/x-patch
Size: 5733 bytes
Desc: not available
URL: <http://lists.webkit.org/pipermail/webkit-gtk/attachments/20120523/28ca3643/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Complete-backport-of-the-File-chooser-signal-to-WebK.patch
Type: text/x-patch
Size: 13687 bytes
Desc: not available
URL: <http://lists.webkit.org/pipermail/webkit-gtk/attachments/20120523/28ca3643/attachment-0003.bin>