Bug 169278

Summary: [GTK] Use GtkFileChooserNative for open/save dialogs
Product: WebKit Reporter: Adrian Perez <aperez>
Component: WebKitGTKAssignee: Adrian Perez <aperez>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, cgarcia, commit-queue, gustavo, mcatanzaro, mrobinson, tpopela
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Adrian Perez 2017-03-07 06:25:59 PST
Starting with GTK+ 3.20 there is a new GtkFileChooserNative file chooser
dialog implementation:

  https://developer.gnome.org/gtk3/stable/gtk3-GtkFileChooserNative.html

It would be interesting to use it if possible, when building with GTK+ 3.20+
because it allows for a few niceties which better integrate applications
using WebKitGTK+ into the running environment:

 - When an application is running on a desktop environment which does not
   use GTK+, the desktop environment could set up things to use its native
   open/save dialogs (e.g. the Epiphany running on KDE could use Qt dialogs).

 - When an application is running inside a sandboxed environment (for example
   with Flatpak — http://flatpak.org), “portals” may be used automatically
   to allow for selection of files from outside of the sandboxed environment.

 - Using platform-specific open/save dialogs where available (e.g. in Mac OS;
   at the moment it is not possible to build a recent WebKitGTK+ for Windows,
   but the code would be ready to use Windows systems dialogs if somebody
   would add the other missing pieces).

Note that GtkFileChooserNative has a few shortcomings when compared to
GtkFileChooserDialog, so part of the work is to evaluate whether the current
uses of the latter can be replaced with GtkFileChooserNative.
Comment 1 Michael Catanzaro 2017-03-07 06:37:30 PST
Looks pretty easy, just need to update webkitWebViewRunFileChooser in WebKitWebView.cpp.

Looks like it has no support for MIME types filter, though? That's a shame.
Comment 2 Adrian Perez 2017-03-07 07:00:29 PST
(In reply to comment #1)
> Looks pretty easy, just need to update webkitWebViewRunFileChooser in
> WebKitWebView.cpp.
> 
> Looks like it has no support for MIME types filter, though? That's a shame.

I am already making a test build with locally with the needed changes
applied; I will attach it to the bug soon.

Regarding the filters, from the documentation (linked in the bug
description) my interpretation is that:

 - MIME-type based filters are supported by the Flatpak portal chooser
   implementation. We are only using this kind of filter in WebKitGTK+
   (good!).

 - No filters at all are supported in the Windows implementation. When
   using filters GtkFileChooserNative automatically falls-back to using
   GtkFileChooserDialog. Not like we care right now, as we are lacking
   in the Windows support department, but if somebody made WebKitGTK+
   work in Windows, it seems reasonable to use GtkFileChooserNative and
   let it trigger the fall-back code path when needed.

TL;DR: We will be fine as we are only using MIME-type based filters.
Comment 3 Adrian Perez 2017-03-07 08:10:45 PST
Created attachment 303653 [details]
Patch
Comment 4 WebKit Commit Bot 2017-03-07 08:12:28 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 5 Michael Catanzaro 2017-03-07 11:01:16 PST
Comment on attachment 303653 [details]
Patch

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

LGTM. Carlos Garcia will want to review it as well.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:569
> +    // XXX: Using a filter based on MIME types does not work in Windows. If such a filter is
> +    //      set, GtkFileChooserNative automatically falls-back to using GtkFileChooserDialog.

That's not our problem. We don't even support Windows. You can remove this comment.
Comment 6 Michael Catanzaro 2017-03-07 11:01:37 PST
Note: this is required for our goal to release Flatpaks.
Comment 7 Adrian Perez 2017-03-08 09:53:22 PST
Created attachment 303817 [details]
Patch
Comment 8 Adrian Perez 2017-03-08 09:54:10 PST
(In reply to comment #5)
> Comment on attachment 303653 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=303653&action=review
> 
> LGTM. Carlos Garcia will want to review it as well.
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:569
> > +    // XXX: Using a filter based on MIME types does not work in Windows. If such a filter is
> > +    //      set, GtkFileChooserNative automatically falls-back to using GtkFileChooserDialog.
> 
> That's not our problem. We don't even support Windows. You can remove this
> comment.

Comment removed. It should be good to go now :-)
Comment 9 WebKit Commit Bot 2017-03-09 05:42:42 PST
Comment on attachment 303817 [details]
Patch

Clearing flags on attachment: 303817

Committed r213637: <http://trac.webkit.org/changeset/213637>
Comment 10 WebKit Commit Bot 2017-03-09 05:42:46 PST
All reviewed patches have been landed.  Closing bug.