Skip to content

Conversation

@SyntevoAlex
Copy link
Member

@SyntevoAlex SyntevoAlex commented Jul 9, 2022

Fixes #247

The problem was introduced in patch for Bug 541635, which accidentally
ignored calculated dragging variable, causing drag to begin
immediately when mouse is moved.

Also, it introduced a crazy loop, which, if it proceeds to second
iteration, will loop forever, because GTK events can't be processed when
main thread is busy running the infinite loop. Luckily, it seems that
the loop always exited on first iteration, and its meaning was simply to
return false when mouse is not down.

The reason why drag&drop can become stuck is not exactly clear, but I'm
confident that it was related to very short drag&drop mouse move
threshold (basically, 1px). With the regular threshold, the bug may
still be present, but it's almost impossible to reproduce.

The real case behind this bug is that when actual human double-clicks a
TreeItem, sometimes mouse will be moved very slightly when doing so,
which started a drag&drop, which got stuck and caused further problems
in applications which were puzzled by never ending drag&drop. Also, it
results in TreeDragSourceEffect image leak, because it never receives
the drag end event.

@SyntevoAlex SyntevoAlex force-pushed the z_alexandr.miloslavskiy/#0497_SWT_Issue0247_DragDropStuck branch from c2ee77b to 03bb4ee Compare July 9, 2022 02:08
@github-actions
Copy link
Contributor

github-actions bot commented Jul 9, 2022

Unit Test Results

     602 files       602 suites   14m 45s ⏱️
  4 013 tests   4 002 ✔️     5 💤   5  1 🔥
24 022 runs  23 874 ✔️ 134 💤 10  4 🔥

For more details on these failures and errors, see this check.

Results for commit d1c274e.

♻️ This comment has been updated with latest results.

@SyntevoAlex SyntevoAlex force-pushed the z_alexandr.miloslavskiy/#0497_SWT_Issue0247_DragDropStuck branch 2 times, most recently from 058d5a3 to d1c274e Compare July 9, 2022 02:55
@SyntevoAlex
Copy link
Member Author

@iloveeclipse could you please review?

@iloveeclipse
Copy link
Member

@SyntevoAlex : I still have RHEL 7.9 and no Wayland, so I can't really review that.
@joel-majano: you seem to have Wayland (at least GTK4 means you have more modern environment) : would you please take a look?

@joel-majano
Copy link
Contributor

@iloveeclipse just took a look.

Comparing this patch against master there is a very noticeable decrease in the amount of times that dragStart/dragFinish show up in the console when clicking rapidly while moving the mouse slowly. There are instances where I can trigger the bug still, where only dragStart shows up. However, the bug does seem to have decreased in frequency significantly, and the greatly reduced amount of drags triggered seems like a more desired outcome anyway.

@SyntevoAlex
Copy link
Member Author

when clicking rapidly while moving the mouse slowly

Either it's not "rapidly" or not "slowly" :)

GTK requires mouse to move 8px from starting point in order for gtk_drag_check_threshold() to detect drag.

When I move mouse slowly and click rapidly, with the patch, drag&drop never begins. When I move mouse faster, bug is still reproducible. But this is not something people would do. The original bug was easily triggered when I intended to simply double-click a TreeItem and didn't intend to do weird things.

So it can be said that the patch prevents accidental drag&drop when double clicking, and by this, it also prevents stuck drag&drop when double clicking for realistic purposes.

@joel-majano
Copy link
Contributor

The original bug was easily triggered when I intended to simply double-click a TreeItem and didn't intend to do weird things.

Agreed. For all realistic cases, this patch resolves the issue. I had to do some mouse wizardry (clicking the mouse waay too fast...) to trigger the bug.

@SyntevoAlex
Copy link
Member Author

You mean, move mouse too fast?
Because it has to move 8px between two clicks in order to begin drag&drop.
Or I'm not understanding something in your environment.

@SyntevoAlex
Copy link
Member Author

Reminder: PR is still not merged.

@SyntevoAlex
Copy link
Member Author

@akurtakov is there anything that block this PR from being merged?

@akurtakov
Copy link
Member

I just haven't looked into it. If you're confident, just merge it.

@SyntevoAlex
Copy link
Member Author

OK, let's merge it, then.

The problem was introduced in patch for Bug 541635, which accidentally
ignored calculated `dragging` variable, causing drag to begin
immediately when mouse is moved.

Also, it introduced a crazy loop, which, if it proceeds to second
iteration, will loop forever, because GTK events can't be processed when
main thread is busy running the infinite loop. Luckily, it seems that
the loop always exited on first iteration, and its meaning was simply to
`return false` when mouse is not down.

The reason why drag&drop can become stuck is not exactly clear, but I'm
confident that it was related to very short drag&drop mouse move
threshold (basically, 1px). With the regular threshold, the bug may
still be present, but it's almost impossible to reproduce.

The real case behind this bug is that when actual human double-clicks a
TreeItem, sometimes mouse will be moved very slightly when doing so,
which started a drag&drop, which got stuck and caused further problems
in applications which were puzzled by never ending drag&drop. Also, it
results in `TreeDragSourceEffect` image leak, because it never receives
the drag end event.

Signed-off-by: Alexandr Miloslavskiy <[email protected]>
@SyntevoAlex SyntevoAlex force-pushed the z_alexandr.miloslavskiy/#0497_SWT_Issue0247_DragDropStuck branch from d1c274e to 1234b88 Compare August 9, 2022 16:30
@SyntevoAlex SyntevoAlex merged commit 1bb1016 into eclipse-platform:master Aug 9, 2022
@lshanmug lshanmug added this to the 4.25 M3 milestone Aug 17, 2022
@SyntevoAlex SyntevoAlex deleted the z_alexandr.miloslavskiy/#0497_SWT_Issue0247_DragDropStuck branch August 21, 2022 11:52
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.

[GTK][Wayland] Double clicking sometimes results in an unfinished drag&drop

5 participants