Nothing Special   »   [go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

DND does not finish on desktop #1673

Closed
tokyo4j opened this issue Apr 2, 2024 · 3 comments · Fixed by #1687
Closed

DND does not finish on desktop #1673

tokyo4j opened this issue Apr 2, 2024 · 3 comments · Fixed by #1687
Labels
bug Something isn't working help wanted Extra attention is needed
Milestone
0.7.2

Comments

@tokyo4j
Copy link
Contributor
tokyo4j commented Apr 2, 2024

DND doesn't finish properly when the cursor button is released on desktop.

dnd-on-desktop.mp4

I confirmed removing the check for ctx.surface in cursor_button_release() (introduced in a252308) fixed the problem, though I don't know the purpose of this check.

diff --git a/src/input/cursor.c b/src/input/cursor.c
index eb359e7..4a9f6b9 100644
--- a/src/input/cursor.c
+++ b/src/input/cursor.c
@@ -1067,7 +1067,7 @@ cursor_button_release(struct seat *seat, uint32_t button,
        bool consumed_by_frame_context =
                handle_release_mousebinding(server, &ctx, button);
 
-       if (ctx.surface && !consumed_by_frame_context) {
+       if (!consumed_by_frame_context) {
                /* Notify client with pointer focus of button release */
                wlr_seat_pointer_notify_button(seat->seat, time_msec,
                        button, button_state);
@Consolatis
Copy link
Member

Agree, that check doesn't appear to make sense for the cursor button release handler.

The commit linked is part of the original move to scene graph.
I assume what happened was that that condition is basically a copy from cursor_button_press() where the check likely makes sense.

CC @johanmalm just to ensure we don't miss any context here.

@Consolatis Consolatis added bug Something isn't working help wanted Extra attention is needed labels Apr 2, 2024
@jp7677
Copy link
Contributor
jp7677 commented Apr 3, 2024

Nice find. The same issue can also be seen in firefox when dragging a tab out of firefox. Would be cool to have this addressed.

@Consolatis Consolatis added this to the 0.7.2 milestone Apr 3, 2024
@johanmalm
Copy link
Collaborator

Agree, that check doesn't appear to make sense for the cursor button release handler.

The commit linked is part of the original move to scene graph. I assume what happened was that that condition is basically a copy from cursor_button_press() where the check likely makes sense.

CC @johanmalm just to ensure we don't miss any context here.

I'm fine with this patch. Can't think of why it's there either. Like you say - quite possibly cruft from pre-scene-graph.

I've tested with a few scenarios with scroll bars and selected text - but all seems fine.

johanmalm added a commit to johanmalm/labwc that referenced this issue Apr 6, 2024
@johanmalm
cursor: fix dnd bug
be2ead2 
...where dnd does not finish properly on cursor-button-release if there
is no surface under the cursor such as on the desktop when no background
client is running.

Written-by: @tokyo4j

Fixes: labwc#1673
@johanmalm johanmalm mentioned this issue Apr 6, 2024
Merged
johanmalm added a commit that referenced this issue Apr 6, 2024
@johanmalm
cursor: fix dnd bug
3385913 
...where dnd does not finish properly on cursor-button-release if there
is no surface under the cursor such as on the desktop when no background
client is running.

Written-by: @tokyo4j

Fixes: #1673
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Milestone
0.7.2
Development

Successfully merging a pull request may close this issue.

cursor: fix dnd bug johanmalm/labwc
4 participants
@johanmalm @jp7677 @tokyo4j @Consolatis