[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.

Todos:

• 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:

  https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/508

Commit Summary

• Fix a typo
• Comment out suspicious line
• Merge branch 'Cog' into sqUnixXdnd
• Refactor changes
• Don't skip SQDragLeave if XGetSelectionOwner failed

File Changes

• M platforms/unix/vm-display-X11/sqUnixXdnd.c (11)

Patch Links:

• https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/508.patch
• https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/508.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

@LinqLover pushed 1 commit.

• e2be1e8 Don't skip SQDragLeave if XGetSelectionOwner failed

—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.

Ehm, all the pharo builds are failing. Is this my fault?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

@nicolas-cellier-aka-nice Would you mind to have a short look at this? :-)

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

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 are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

@LinqLover pushed 2 commits.

• aafdc28 Only record SQDragLeave xor SQDragDrop
• 4def9a6 Merge remote-tracking branch 'origin/Cog' into sqUnixXdnd

—
You are receiving this because you are subscribed to this thread.
View it on GitHub or unsubscribe.

Thank you for the detailed explanations, @nicolas-cellier-aka-nice!

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. :-)

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Regarding code ownership - there currently is nobody specifically tending to the unix VM, or the X window display system. Most of the code is more than 15 years old, written by Ian Piumarta. Others and I contributed smaller parts over the years. That history got lost when switching from svn to git.

As for DND, at some point around 2007 it was working fully - you could drag pictures and rich text into Squeak (e.g. a paragraph from Wikipedia), and you could drag e.g. a morph out of Squeak into an office document (where it would appear as a bitmap image) or onto another Squeak (where it would appear as fully functioning object, all the scripts etc. were being serialized). The latter part might be why there are squeak-specific DnD events.

This was used for full interoperability inside of the Sugar OS on the OLPC laptop. E.g. you could drag a morph out of Etoys onto the Sugar Frame Clipboard, switch apps, and drag it into the other app. I used to be the maintainer of Squeak/Etoys for Sugar but haven’t touched it in years.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

@codefrau This is very interesting to hear! For starters, my goal is to translate every incoming drag into a transfer morph that can be handled natively by any morph. However, reviving the full potential of DnD will be a very interesting follow-up project. If you have further information on how this could break all without anyone noticing, please tell me. :-) Do you know whether all platforms already supported this?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

I don't remember if all platforms supported this. Your best bet is to try with Etoys from Squeakland. This one here includes VMs for Win, Mac, and Linux: http://www.squeakland.org/content/installers/Etoys-To-Go-5.0.zip

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Which VM version is that? On Windows, DnD works as well as in the latest OSVM. Unfortunately, I cannot find a 64 bit version for Linux, so I cannot test Etoys To Go on Linux ...

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Hey @LinqLover, you should be able to get it to run on a 64bit linux as well. When running it will report which libraries are currently not installed in 32bit, on my system this was the only step necessary to get it to run: sudo apt install libxrender1:i386 libpangocairo-1.0-0:i386

In general, once you know which libraries cannot be loaded, you can run for example ldd Contents/Linux-i686/so.vm-display-X11 to see which of its dependencies are missing and then install those via apt-get with the :i386 suffix.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

@tom95 The message only I'm seeing is:

$ ./etoys.sh
./etoys.sh: line 107: ./Contents/Linux-i686/etoys: cannot execute binary file: Exec format error

ldd gives me not a dynamic executable. Does native Linux support more backward compatibility in terms of architecture than a virtualized Linux? I am using Ubuntu in Windows Subsystem for Linux.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Oh okay, in that case this might be your best bet: https://superuser.com/questions/1407730/run-32-bit-application-on-ubuntu-on-windows-subsystem-for-linux

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

@tom95 Great to know that WSL2 supports this. I still have to set up this, thank you :)

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Inside Etoys-To-Go are old 32 Bit interpreter VMs, from before Cog was around. You need those to be able to run the Etoys image. On Linux, there used to be a simple way to install the 32 bit support even on a 64 bit system, but I don’t know what the current status is.

That Etoys image might have some more support code for DnD handling than current Squeak. Not everything has been properly integrated back. @timfel did the bulk of that work.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.