[webkit-reviews] review denied: [Bug 212322] [GTK4] Implement file chooser : [Attachment 400723] Patch

[bugzilla-daemon at webkit.org]

Adrian Perez <aperez at igalia.com> has denied Santosh Mahto
<santosh.mahto at collabora.com>'s request for review:
Bug 212322: [GTK4] Implement file chooser
https://bugs.webkit.org/show_bug.cgi?id=212322

Attachment 400723: Patch

https://bugs.webkit.org/attachment.cgi?id=400723&action=review




--- Comment #4 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 400723
  --> https://bugs.webkit.org/attachment.cgi?id=400723
Patch

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

> Source/WebCore/ChangeLog:7
> +	   to port FileChooser seemlessly.

Typo: seemlessly → seamlessly

> Source/WebCore/platform/gtk/GtkVersioning.h:166
> +	   return gtk_file_chooser_select_file(chooser,
g_file_new_for_path(filename), NULL);

This leaks the GFile returned by g_file_new_for_path(), please store its return
value in a GRefPtr<GFile> before passing it to gtk_file_chooser_select_file()
to
ensure that it will be unref'd properly.

> Source/WebCore/platform/gtk/GtkVersioning.h:178
> +	   gchar* uri = g_file_get_path(static_cast<GFile*>(node->data));

The documentation for gtk_file_chooser_get_filenames() says:

   “If files in the current folder cannot be represented as
    local filenames they will be ignored.“

Therefore we need to check here whether the GFile is local or
not, and skip adding the non-local ones to the result list. I
think that using g_file_is_native() would fit well.