[webkit-gtk] Implement runOpenPanel in WebKit2GTK+

Carlos Garcia Campos cgarcia at igalia.com
Sat Feb 11 10:20:51 PST 2012


El sáb, 11-02-2012 a las 00:10 +0100, Mario Sanchez Prada escribió:
> Hi all,
> 
> I started yesterday working on an implementation for WebKitUIClient's
> runOpenPanel() in WebKit2GTK+, and would like to share here the ideas
> behind it and the current status of it (aka the patch I have so far).
>
> What I did so far is basically the following:
> 
> - Implement runOpenPanel() in WebKitUIClient.cpp, where I create 
>   an object of a new class wrapping WKOpenPanelResultListenerRef and
>   WKOpenPanelParametersRef, which I named WebKitOpenPanelDecision atm.
> 
> - The API for that new WebKitOpenPanelDecision is simple: it provides
> a 
>   allows_multiple_files(), choose_files() and cancel() methods, 
>   implemented in terms of the wrapped WKOpenPanelResultListenerRef and
>   WKOpenPanelParametersRef attributes.

Decision is not probably the best name, even more if the object wraps
both the listener and the parameters. I think something like Operation
or Request could sound better. But I like the idea of using a single
object to wrap both. 

You are ignoring the accepted MIME types, you could add a method that
returns a GtkFileFilter with the mime types. Also the selected files,
even though there isn't C API for that, they are in
WebOpenPanelParameters object, so you can get them with
toImpl(wkParameters)->selectedFileNames(). 

> - Once that object is created, pass it to the webView to emit a new 
>   signal from there, including that new object as parameter. At the
>   moment, I named that new signal 'decide-open-panel', inspired in the
>   'decide-policy' signal. It uses g_signal_accumulator_true_handled.

In this case it's not a decision, but a request to open the file chooser
to select files to upload. It's similar to WebKitWebView::script-* or
WebKitWebView::create signals. And "open-panel" sounds too mac,
run-filechooser sounds more gnome.

> - Provide a default handler in the webview for this new signal so, if 
>   not intercepted from the app, a GtkFileChooserDialog will be run and
>   a handler for that dialog's 'response' signal will be installed, to 
>   asynchronously handle the result after selecting files/cancelling:
> it 
>   will call webkit_open_panel_decision_cancel() if the dialog was
>   cancelled it and webkit_open_panel_decision_choose_files()
> otherwise.
> 
> I'm not quite sure about some parts of this at the moment (e.g. the
> names for signals and the new class), but I think this is a good point
> to start discussing about it, and see if we can agree on something
> before keeping working along this line.

For the signal I would use something like run-filechooser, and for the
helper object, WebKitFileChooserOperation or WebKitFileChooserRequest.

> So, I'm attaching the patch I've at the moment for reference and in
> order to help with the discussion. Please notice it's not a final
> patch,
> but more kind of a draft :-)

Ok, even though it's not a final patch I'll add some initial comments
that I hope will help you to write the final patch.

> Mario
> 
> 

> +#include "config.h"
> +#include "WebKitOpenPanelDecision.h"
> +
> +#include "APIObject.h"
> +#include "FileSystem.h"
> +#include "ImmutableArray.h"
> +#include "WKOpenPanelResultListener.h"
> +#include "WKOpenPanelParameters.h"

For C API headers, use #include <WebKit2/Foo.h> and move them to
WebKitPrivate.h

> +#include "WebKitOpenPanelDecisionPrivate.h"
> +#include "WebKitPrivate.h"
> +
> +using namespace WebKit;
> +
> +/**
> + * SECTION: WebKitOpenPanelDecision
> + * @Short_description: An open panel decision
> + * @Title: WebKitOpenPanelDecision
> + * @See_also: #WebKitWebView
> + */
> +G_DEFINE_TYPE(WebKitOpenPanelDecision, webkit_open_panel_decision, G_TYPE_OBJECT)
> +
> +struct _WebKitOpenPanelDecisionPrivate {
> +    WKRetainPtr<WKOpenPanelParametersRef> parameters;
> +    WKRetainPtr<WKOpenPanelResultListenerRef> listener;

Use wk prefix for variable names that are C API objects, wkParameters
and wkListener in this case.


> +static void webkitOpenPanelDecisionFinalize(GObject* object)
> +{
> +     WebKitOpenPanelDecisionPrivate* priv = WEBKIT_OPEN_PANEL_DECISION(object)->priv;
> +    priv->~WebKitOpenPanelDecisionPrivate();
> +    G_OBJECT_CLASS(webkit_open_panel_decision_parent_class)->finalize(object);
> +}

If choose_files or cancel is not called when the object finalizes, we
could call cancel() to make sure the operation finishes. We do something
similar with the policy decision. It can happen if the user handles the
signal and just returns TRUE, to avoid any filechooser to be shown. 

> +gboolean webkit_open_panel_decision_allows_multiple_files(WebKitOpenPanelDecision* decision)
> +{

Use g_return macros in public methods.

> +    return WKOpenPanelParametersGetAllowsMultipleFiles(decision->priv->parameters.get());
> +}
> +
> +void webkit_open_panel_decision_choose_files(WebKitOpenPanelDecision* decision, GSList* fileUris)
> +{

Ditto.

> +    Vector<RefPtr<APIObject> > selectedUris;
> +    int i = 0;
> +    for (GSList* item = fileUris ; item ; item = item->next) {
> +        if (!item->data)
> +            continue;
> +        selectedUris.append(WebURL::create(WebCore::filenameToString(static_cast<char*>(item->data))));

List items must be uris, not filenames. 

> +    }
> +
> +    WKOpenPanelResultListenerChooseFiles(decision->priv->listener.get(), toAPI(ImmutableArray::adopt(selectedUris).get()));

This can be implemented using the C API:

WKRetainPtr<WKMutableArrayRef> wkSelectedURIs(AdoptWK, WKMutableArrayCreate());

for (GSList* item = fileURIs ; item ; item = g_slist_next(item)) {
    if (!item->data)
        continue;
    WKRetainPtr<WKURLRef> wkURL(AdoptWK, WKURLCreateWithUTF8CString(static_cast<char*>(item->data)));
    WKArrayAppendItem(wkSelectedURIs.get(), wkURL.get());
}

WKOpenPanelResultListenerChooseFiles(decision->priv->listener.get(),  wkSelectedURIs.get());

It looks a bit easier to read to me, but your code is fine too.

> +}
> +
> +void webkit_open_panel_decision_cancel (WebKitOpenPanelDecision* decision)
> +{

g_return.

> +    WKOpenPanelResultListenerCancel(decision->priv->listener.get());
> +}

> diff --git a/Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp b/Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp
> index a659ad3..a051eaa 100644
> --- a/Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp
> +++ b/Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp
> @@ -20,12 +20,17 @@
>  #include "config.h"
>  #include "WebKitUIClient.h"
>  
> +#include "WKOpenPanelParameters.h"

This should already be included by WebKitPrivate.h.

> +#include "WebKitOpenPanelDecision.h"
> +#include "WebKitOpenPanelDecisionPrivate.h"
>  #include "WebKitPrivate.h"
>  #include "WebKitWebViewBasePrivate.h"
>  #include "WebKitWebViewPrivate.h"
>  #include "WebKitWindowPropertiesPrivate.h"
>  #include "WebPageProxy.h"
>  #include <WebCore/GtkUtilities.h>
> +#include <glib/gi18n-lib.h>
> +#include <wtf/gobject/GRefPtr.h>
>  
>  using namespace WebKit;
>  
> @@ -126,6 +131,12 @@ static void setWindowFrame(WKPageRef page, WKRect frame, const void* clientInfo)
>      webkitWindowPropertiesSetGeometry(windowProperties, &geometry);
>  }
>  
> +static void runOpenPanel(WKPageRef page, WKFrameRef frame, WKOpenPanelParametersRef parameters, WKOpenPanelResultListenerRef listener, const void *clientInfo)
> +{
> +    GRefPtr<WebKitOpenPanelDecision> decision = adoptGRef(webkitOpenPanelDecisionCreate(parameters, listener));
> +    webkitWebViewMakeOpenPanelDecision(WEBKIT_WEB_VIEW(clientInfo), WEBKIT_OPEN_PANEL_DECISION(decision.get()));

No need to cast, decision is already a WebKitOpenPanelDecision.

> +}
> +

> diff --git a/Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp b/Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp
> index c3acccc..21a7a16 100644
> --- a/Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp
> +++ b/Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp
> @@ -21,6 +21,8 @@
>  #include "config.h"
>  #include "WebKitWebView.h"
>  
> +#include "FileSystem.h"
> +#include "GOwnPtr.h"

Use the angle-bracket form for header files from other WebCore and
JavaScriptCore, see
http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

>  
> +static void fileChooserDialogResponseCallback(GtkDialog* dialog, gint responseID, WebKitOpenPanelDecision* decision)
> +{

You can adopt the ref here and use an early return.

GRefPtr<WebKitOpenPanelDecision> decision = adoptGRef(decision);
if (responseID != GTK_RESPONSE_ACCEPT) {
    webkit_open_panel_decision_cancel(decision);
    gtk_widget_destroy(GTK_WIDGET(dialog));
    return;
}

> +    if (responseID == GTK_RESPONSE_ACCEPT) {
> +        GSList* fileUris = NULL;
> +        if (gtk_file_chooser_get_select_multiple(GTK_FILE_CHOOSER(dialog))) {
> +            fileUris = gtk_file_chooser_get_uris(GTK_FILE_CHOOSER(dialog));
> +        } else {
> +            gchar* fileUri = gtk_file_chooser_get_uri(GTK_FILE_CHOOSER(dialog));
> +            if (!fileUri)
> +                fileUris = g_slist_append(fileUris, fileUri);
> +        }

I think gtk_file_chooser_get_uris works also if select multiple is not
allowed, have you tried it?

> +        webkit_open_panel_decision_choose_files(decision, fileUris);
> +        g_slist_foreach(fileUris, reinterpret_cast<GFunc>(g_free), 0);
> +        g_slist_free(fileUris);

g_list_free_full() does both, but you should use GOwnPtr anyway.

> +
> +    } else
> +        webkit_open_panel_decision_cancel(decision);
> +
> +    g_object_unref(decision);
> +
> +    gtk_widget_destroy(GTK_WIDGET(dialog));
> +}
> +
> +static gboolean webkitWebViewDecideOpenPanel(WebKitWebView* webView, WebKitOpenPanelDecision* decision)
> +{
> +    GtkWidget* toplevel = gtk_widget_get_toplevel(GTK_WIDGET(webView));
> +    if (!WebCore::widgetIsOnscreenToplevelWindow(toplevel))
> +        toplevel = 0;

WebKitWebView has 'using namespace WebCore;' so no need for WebCore::

> +    GtkWidget* dialog = gtk_file_chooser_dialog_new(_("Upload File"),

I would use something lile Select Files or Select Files to upload, maybe
using File or Files depending on the allow_multiple parameter. Upload
File makes me think the dialog will upload a file, while it actually
select a file that might be uploaded later.

> +                                                    toplevel ? GTK_WINDOW(toplevel) : 0,
> +                                                    GTK_FILE_CHOOSER_ACTION_OPEN,
> +                                                    GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
> +                                                    GTK_STOCK_OPEN, GTK_RESPONSE_ACCEPT,
> +                                                    NULL);
> +
> +    gtk_file_chooser_set_select_multiple(GTK_FILE_CHOOSER(dialog), webkit_open_panel_decision_allows_multiple_files(decision));
> +    g_signal_connect(G_OBJECT(dialog), "response", G_CALLBACK(fileChooserDialogResponseCallback), g_object_ref(decision));

g_signal_connect receives a gpointer, you don't need the G_OBJECT cast.

> +    gtk_widget_show(dialog);
> +
> +    return TRUE;
> +}
> +

Don't forget to include the new header in the main header webkit2.h

-- 
Carlos Garcia Campos
http://pgp.rediris.es:11371/pks/lookup?op=get&search=0xF3D322D0EC4582C3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.webkit.org/pipermail/webkit-gtk/attachments/20120211/3e6072d0/attachment.bin>


More information about the webkit-gtk mailing list