Skip to content

[GTK] Receive dropped files using the File Transfer portal#25575

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
GeorgesStavracas:eng/GTK-Drag-and-drop-does-not-work-under-flatpak
Mar 11, 2024
Merged

[GTK] Receive dropped files using the File Transfer portal#25575
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
GeorgesStavracas:eng/GTK-Drag-and-drop-does-not-work-under-flatpak

Conversation

@GeorgesStavracas
Copy link
Copy Markdown
Contributor

@GeorgesStavracas GeorgesStavracas commented Mar 7, 2024

ff20e45

[GTK] Receive dropped files using the File Transfer portal
https://bugs.webkit.org/show_bug.cgi?id=212079

Reviewed by Carlos Garcia Campos.

GTK4 has content serializers and deserializers that are able to read
from, and write to, the File Transfer portal. They're used when using
the high level content formats API, i.e. using GTypes. In particular,
GTK4 is able to convert from "application/vnd.portal.filetransfer" (and
the legacy "application/vnd.portal.files") from and into GdkFileLists.

The DropTargetGtk4 code did not advertise the GdkFileList GType as a
potential data offering, so GTK4 was never able to let the File Transfer
portal conversion happen.

Add the GdkFileList GType to the content formats.

This creates a small conflict: many apps, e.g. Nautilus, advertise files
both as "application/vnd.portal.filetransfer", and as "text/uri-list".
The file URIs under "text/uri-list" are meaningless to sandboxed apps,
as they might be in innaccessible locations. Which is why I've made the
executive decision to ignore "file://" URIs when the File Transfer portal
is used. Other types of URIs are preserved.

(Other browsers take a more radical approach of ignoring all URIs on the
presence of the File Transfer portal vendor.)

* Source/WebKit/UIProcess/API/gtk/DropTarget.h:
* Source/WebKit/UIProcess/API/gtk/DropTargetGtk4.cpp:
(WebKit::DropTarget::DropTarget):
(WebKit::DropTarget::accept):
(WebKit::DropReadAsyncData::DropReadAsyncData):
(WebKit::DropTarget::loadData):
(WebKit::DropTarget::didLoadData):

Canonical link: https://commits.webkit.org/275908@main

e7049ee

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style 🛠 ios 🛠 mac 🛠 wpe 🛠 wincairo
✅ 🛠 ios-sim 🛠 mac-AS-debug 🧪 wpe-wk2
🧪 webkitperl 🧪 ios-wk2 🧪 api-mac 🧪 api-wpe
🧪 ios-wk2-wpt 🧪 mac-wk1 🛠 wpe-skia
🧪 api-ios 🧪 mac-wk2 🛠 gtk
🛠 tv 🧪 mac-AS-debug-wk2 🧪 gtk-wk2
🛠 tv-sim 🧪 mac-wk2-stress 🧪 api-gtk
🛠 watch
✅ 🛠 🧪 unsafe-merge 🛠 watch-sim

@GeorgesStavracas GeorgesStavracas requested a review from a team as a code owner March 7, 2024 12:43
@GeorgesStavracas GeorgesStavracas self-assigned this Mar 7, 2024
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Mar 7, 2024

@GeorgesStavracas GeorgesStavracas added the WebKitGTK Bugs related to the Gtk API layer. label Mar 7, 2024
Comment on lines 86 to 87
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really dislike this. Suggestions welcomed.

I tried adding SelectionData::addGFiles but for apparently there must be a corresponding file:// URI for every reported file.

Comment on lines 216 to 231
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this kind of modification / sanitization is allowed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand what you are doing here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block of code basically filters out file://s from the URI list.

The URI list comes as a continuous string separated by \n. So I break the URI string by newline; filter out file:// URIs; and rebuild the (continuous) list in m_uriListBuilder.

After both mimetypes and the GdkFileList are processed, I merge the GFiles as file:// URIs into the URI list.

Comment on lines 216 to 231
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand what you are doing here.

@GeorgesStavracas GeorgesStavracas force-pushed the eng/GTK-Drag-and-drop-does-not-work-under-flatpak branch from 00b3d58 to d51e8fd Compare March 7, 2024 15:33
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Mar 7, 2024

@GeorgesStavracas GeorgesStavracas force-pushed the eng/GTK-Drag-and-drop-does-not-work-under-flatpak branch from d51e8fd to ff4a631 Compare March 8, 2024 14:40
@GeorgesStavracas GeorgesStavracas added the request-merge-queue Request a pull request to be added to merge-queue once ready label Mar 8, 2024
@carlosgcampos carlosgcampos added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 11, 2024
@GeorgesStavracas GeorgesStavracas removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 11, 2024
@GeorgesStavracas GeorgesStavracas force-pushed the eng/GTK-Drag-and-drop-does-not-work-under-flatpak branch from ff4a631 to e7049ee Compare March 11, 2024 14:34
@GeorgesStavracas GeorgesStavracas removed the request-merge-queue Request a pull request to be added to merge-queue once ready label Mar 11, 2024
@TingPing TingPing added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 11, 2024
https://bugs.webkit.org/show_bug.cgi?id=212079

Reviewed by Carlos Garcia Campos.

GTK4 has content serializers and deserializers that are able to read
from, and write to, the File Transfer portal. They're used when using
the high level content formats API, i.e. using GTypes. In particular,
GTK4 is able to convert from "application/vnd.portal.filetransfer" (and
the legacy "application/vnd.portal.files") from and into GdkFileLists.

The DropTargetGtk4 code did not advertise the GdkFileList GType as a
potential data offering, so GTK4 was never able to let the File Transfer
portal conversion happen.

Add the GdkFileList GType to the content formats.

This creates a small conflict: many apps, e.g. Nautilus, advertise files
both as "application/vnd.portal.filetransfer", and as "text/uri-list".
The file URIs under "text/uri-list" are meaningless to sandboxed apps,
as they might be in innaccessible locations. Which is why I've made the
executive decision to ignore "file://" URIs when the File Transfer portal
is used. Other types of URIs are preserved.

(Other browsers take a more radical approach of ignoring all URIs on the
presence of the File Transfer portal vendor.)

* Source/WebKit/UIProcess/API/gtk/DropTarget.h:
* Source/WebKit/UIProcess/API/gtk/DropTargetGtk4.cpp:
(WebKit::DropTarget::DropTarget):
(WebKit::DropTarget::accept):
(WebKit::DropReadAsyncData::DropReadAsyncData):
(WebKit::DropTarget::loadData):
(WebKit::DropTarget::didLoadData):

Canonical link: https://commits.webkit.org/275908@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/GTK-Drag-and-drop-does-not-work-under-flatpak branch from e7049ee to ff20e45 Compare March 11, 2024 16:12
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 275908@main (ff20e45): https://commits.webkit.org/275908@main

Reviewed commits have been landed. Closing PR #25575 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit ff20e45 into WebKit:main Mar 11, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 11, 2024
@GeorgesStavracas GeorgesStavracas deleted the eng/GTK-Drag-and-drop-does-not-work-under-flatpak branch March 11, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebKitGTK Bugs related to the Gtk API layer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants