[OpenSmalltalk/opensmalltalk-vm] sqUnixXdnd: Don't record SQDragLeave when XdndDrop is handled (#508)
A bit of context: For Squeak, I am currently implementing full-fledged handling of DND events sent by the host system. Actually, Squeak only handles SQDragDrop at the moment.
However, when logging all received DND events on the image side, I discovered that whenever a file is dropped into the VM, after the last SQDragMove an SQDragLeave is received before the eventual SQDragDrop. This looks very suspicious to me. Neither does the same happen on any other platform (tested on Win32, too), nor does it appear logical to me in any way. If I release the mouse button over the image, the drag did not leave, so a "drag leave" should not be recorded.
This MR makes sure that in dndInDrop(), an SQDragLeave event is only recorded if the drop was not successful. (Please note that the SQDragDrop event itself will be sent in a deferred manner; dndInDrop() itself only calls XConvertSelection() from the X-Server which then will send a SelectionNotify which will trigger dndGetSelection() and eventually have generateSqueakDropEventIfDroppedFiles() called.)
Please review carefully, as I am a bloody VM newbie and this is my very first commit to this repo! :-) Mentioning @eliotmiranda who appears to be the code owner of this plugin, according to the file's history.
Think about backward compatibility: Squeak did not respect any of the drag events but SQDragDrop until today, and I'm reworking that handling ATM, so there will be no problem. But are there other users of this plugin that could rely on the old behavior which I consider a bug? Or does the sq in the prefix unambiguously stand for Squeak?
Are there any tests I could run to make sure that this change cannot break anything else?
You can view, comment on, or merge this pull request online at:
Concerning Pharo, you did not break anything.
External libraries are precompiled on the CI in some cache.
When version number of those libraries are update, the cache is not automatically cleared.
Therefore, the failure is postponed until the cache is cleared, which happens on a regular schedule apparently.
Concerning sq, we did not want to change each and every prefix to osvm when we switched squeak vm to osvm.
It would have touched each and every file and the VMMaker and all plugin repositories, for not much added value.
Who knows how many regressions could have been un-noticed.
So do not consider sq* as Squeak only.
Concerning DND, it's beyond my skill, I've never studied its logic nor particular sequence of events.
What you say sounds logical, and consistent VM behavior is desirable though.
So if you say that you compiled, used the VM and saw no regression in an official release (say drop a .png in Squeak 5.3), I can do the merge (and squash).
Ideally we should do it on an old version of pharo too (7?), but at one moment, pharo people shall help us to help them, and this is not the trend. The INRIA team forked, and when I request community help on Opensmalltalk-vm-dev ML, I ear nothing but a big silence, I guess no one is interested. Newspeak sounds less like a problem, because they use their own form of FFI for windows system integration rather than plugin.
you say that you compiled, used the VM and saw no regression in an official release (say drop a .png in Squeak 5.3)
Yes, I confirm that.
Newspeak sounds less like a problem, because they use their own form of FFI for windows system integration rather than plugin.
What is about Cuis?
consistent VM behavior is desirable though
Hm, please don't merge yet. I just found further small inconsistencies in this implementation I would like to fix within this PR. I will update the description when I am done and mention you again. :-)