Bug 202894 – [GTK][WebInspector] Support for saving data

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Philippe Normand]

Implementation of WebInspectorProxy::platformSave() is missing.

[Philippe Normand]

Created attachment 380843 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 380843 [details]
Patch

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

> Source/WebKit/UIProcess/gtk/WebInspectorProxyGtk.cpp:467
> +        GTK_WINDOW(gtk_widget_get_toplevel(m_inspectorView)), GTK_FILE_CHOOSER_ACTION_SAVE, "Save", "Cancel"));

You need to check the toplevel before using it, you can use WebCore::widgetIsOnscreenToplevelWindow(toplevel) and pass nullptr if it returns false.

> Source/WebKit/UIProcess/gtk/WebInspectorProxyGtk.cpp:470
> +    gtk_file_chooser_set_do_overwrite_confirmation(chooser, true);

TRUE

> Source/WebKit/UIProcess/gtk/WebInspectorProxyGtk.cpp:471
> +    gtk_file_chooser_set_current_name(chooser, suggestedURL.utf8().data());

This expects a name not a URL.

> Source/WebKit/UIProcess/gtk/WebInspectorProxyGtk.cpp:479
> +            if (!base64Decode(content, out, Base64ValidatePadding))

Do we want to decode UTF-16 here?

> Source/WebKit/UIProcess/gtk/WebInspectorProxyGtk.cpp:486
> +            data = content.utf8().data();

We are saving UTF-8 here.

[Philippe Normand]

Comment on attachment 380843 [details]
Patch

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

>> Source/WebKit/UIProcess/gtk/WebInspectorProxyGtk.cpp:471
>> +    gtk_file_chooser_set_current_name(chooser, suggestedURL.utf8().data());
> 
> This expects a name not a URL.

Actually the argument is not an URL, it's a filename. I'll rename it.

[Adrian Perez]

Comment on attachment 380843 [details]
Patch

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

> Source/WebKit/UIProcess/gtk/WebInspectorProxyGtk.cpp:487
> +            dataLength = strlen(data);

You could use “content.utf8().length()” here instead which is O(1), and avoid
the call to strlen() which is O(n).

[Philippe Normand]

Committed r251069: <https://trac.webkit.org/changeset/251069>

[Philippe Normand]

(In reply to Adrian Perez from comment #4)
> Comment on attachment 380843 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=380843&action=review
> 
> > Source/WebKit/UIProcess/gtk/WebInspectorProxyGtk.cpp:487
> > +            dataLength = strlen(data);
> 
> You could use “content.utf8().length()” here instead which is O(1), and avoid
> the call to strlen() which is O(n).

Indeed, good tip :P