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

At the end of drag and drop, grab and release seat #517

Closed
wants to merge 1 commit into from
Closed

Conversation

zhuyaliang
Copy link
Member

Fix #515

@zhuyaliang
Copy link
Member Author

@cwendling Do you accept this repair method

@zhuyaliang
Copy link
Member Author

@cwendling Looking forward to your review

Comment on lines +4197 to +4210
gboolean viewable = TRUE;
GtkWidget *tmp = parent;
while (tmp)
{
if (!gtk_widget_get_mapped (tmp))
{
viewable = FALSE;
break;
}
tmp = gtk_widget_get_parent (tmp);
}

if (viewable)
xgrab_shell = parent;
Copy link
Member

Choose a reason for hiding this comment

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

What is this nested loop about? Removing it seems logically identical (and working the same) save not being quadratic:

Suggested change
gboolean viewable = TRUE;
GtkWidget *tmp = parent;
while (tmp)
{
if (!gtk_widget_get_mapped (tmp))
{
viewable = FALSE;
break;
}
tmp = gtk_widget_get_parent (tmp);
}
if (viewable)
xgrab_shell = parent;
if (gtk_widget_get_mapped (parent))
xgrab_shell = parent;
else
break;


seat = gdk_display_get_default_seat (display);
gdk_seat_grab (seat, window,
GDK_SEAT_CAPABILITY_ALL, TRUE,
Copy link
Member

Choose a reason for hiding this comment

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

That's probably sufficient

Suggested change
GDK_SEAT_CAPABILITY_ALL, TRUE,
GDK_SEAT_CAPABILITY_ALL_POINTING, TRUE,

@cwendling
Copy link
Member

@zhuyaliang I don't like the "fix", but at least it seems to work.

The root issue seems to be inside src/eggtreemultidnd.c's egg_tree_multi_drag_motion_event(), or possibly in how pcmanfm handles drag events.
IIUC it comes from pcmanfm requesting the drag data during drag motion rather than drag drop, but I'm not sure of the implications.
FWIW, File-Roller suffers from the same issue.

@zhuyaliang zhuyaliang closed this Apr 30, 2024
@cwendling
Copy link
Member

@zhuyaliang are you working on a better fix? I didn't mean to say we mustn't do this, just that it feels like a fairly sad hack. I have however for now no real clue how to fix it properly (save on pcmanfm's side by not querying the data on drag-motion), and probably won't have time to dive into this.
If you feel brave enough to dive in deeper, wonderful! Otherwise, if you feel like it's important to fix this, your hack here seems as good as any -- although it might require extensive testing to see if it has unexpcted side effects.

@zhuyaliang
Copy link
Member Author

@cwendling I suspect that the problem lies within pcmanfm, and I will try to solve it again. If anyone needs this PR during this period, I will open it again
Thank you for your code review and feedback. Due to vacation, I was unable to reply to the message in a timely manner

@zhuyaliang
Copy link
Member Author

zhuyaliang commented May 7, 2024

@cwendling I tried to fix the problem in libfm but I'm not sure if this fix is correct,Please help review the code

@zhuyaliang
Copy link
Member Author

There are other issues with libfm, which may not be the most perfect fix, but I don't want to invest too much time in libfm. My focus is on mate desktop

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.

Drag'n'drop blocks GUI
2 participants