Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[X11] Fix drag & drop may stop receiving files in xwayland #122

Closed
wants to merge 2 commits into from

Conversation

56789a1987
Copy link
Contributor

Fix 2 drag & drop issues observed on KDE Wayland, using XWayland.

Changed XdndFinished event to be sent to the source window.
If send it to the root window, after dropping the first file, subsequent files can't be dropped.

Added handling XdndEnter event to check data type.
If the dragged item isn't a file, then reject dropping. This gives visual cursor feedback and prevents the drop event.
Possible fix for: When an unsupported data type is dropped to the window, subsequent files can't be dropped.

Feel free to pick only the first commit. The first issue is more problematic, and the fix doesn't require too many changes.

I agree that my contributions are licensed under the MIT License.

Observed on KDE Wayland, with Dolphin file manager.
After dropping the first file, subsequent file drops will be rejected.
This adds checking data type at drag enter.
If the dragged item isn't a file, show dropping as rejected.
This prevents receiving unsupported data from dnd.

In KDE Wayland, using XWayland,
fixes dropping unsupported data  result in dnd stuck.
Copy link
Collaborator

@aseprite-bot aseprite-bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

const auto sourceWindow = (::Window)event.xclient.data.l[0];
// Bit 0 = the source supports more than 3 data types
// Check the property XdndTypeList on the source window for the list of all available types.
const bool hasMoreTypes = event.xclient.data.l[1] & 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

warning: implicit conversion 'long' -> 'bool' [readability-implicit-bool-conversion]

Suggested change
const bool hasMoreTypes = event.xclient.data.l[1] & 1;
const bool hasMoreTypes = (event.xclient.data.l[1] & 1) != 0;

// Bit 0 = this window accept the drop
event2.xclient.data.l[1] = 1;
event2.xclient.data.l[4] = XdndActionCopy;
if (g_dndType) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

warning: implicit conversion 'Atom' (aka 'unsigned long') -> 'bool' [readability-implicit-bool-conversion]

Suggested change
if (g_dndType) {
if (g_dndType != 0u) {

// Save the X11 window from where this XdndDrop message came
// from, so then we can send a XdndFinished later.
g_dndSource = (::Window)event.xclient.data.l[0];
if (g_dndType) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

warning: implicit conversion 'Atom' (aka 'unsigned long') -> 'bool' [readability-implicit-bool-conversion]

Suggested change
if (g_dndType) {
if (g_dndType != 0u) {


// Ask for the XdndSelection, we're going to receive the
// dropped items in the SelectionNotify.
XConvertSelection(m_display, XdndSelection, g_dndType, XdndSelection, m_window, time);
Copy link
Collaborator

Choose a reason for hiding this comment

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

warning: no header providing "XConvertSelection" is directly included [misc-include-cleaner]

          XConvertSelection(m_display, XdndSelection, g_dndType, XdndSelection, m_window, time);
          ^

@dacap
Copy link
Member

dacap commented Dec 19, 2024

Hi @56789a1987, thanks for these patches, sorry that I cannot test this (I don't have a KDE desktop at hand), but just in case, the beta branch contains several changes in the DnD implementation, could you please test that branch? (and probably patch/target this PR to the beta):

https://github.com/aseprite/laf/tree/beta
284572b

@56789a1987
Copy link
Contributor Author

the beta branch contains several changes in the DnD implementation, could you please test that branch? (and probably patch/target this PR to the beta)

Tested the beta branch with examples/drag_and_drop, generally works well.
XdndEnter seems have been implemented, so the second commit here can be ignored for now.

Applying #121 fixes receiving the same file issue.
And cherry picking the 1861e3f fixes XWayland not accepting files issue.

Other issues: If an unsupported type (such as text) is dropped, the drop zone remains highlighted.

@56789a1987
Copy link
Contributor Author

Hi @dacap, I have tested the beta branch.
The only patches I's like to make is #121 and 1861e3f mentioned above, for both main and beta branch.

6ac753a is implemented on beta, it can be ignored on both branches.

@dacap
Copy link
Member

dacap commented Dec 26, 2024

Thanks @56789a1987 for the patch, I've just included 1861e3f in main and I'll merge main to beta so the beta branch includes these fixes.

@dacap dacap closed this Dec 26, 2024
@56789a1987 56789a1987 deleted the x11-wayland-dnd-fix branch December 27, 2024 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants