Improve drag-and-drop JS event emulation on Selenium 3

[uuf6429]

I'm not fully aware of the flow of running this driver with Selenium3, but this drag and drop failure seemed very consistent to me, so I tried solving it.

Investigation

I noticed the following:

• we try to simulate drag and drop by triggering javascript events
  • the way I saw it, we do not trigger the full / correct flow of events, so I fixed it - but it does not relate to Selenium 3.
• in case of drag & dropping an element onto itself, Selenium 3 had some funny stuff happening with mouse clicks - essentially it doesn't like mousedown/mouseup on an element that has been hidden.
  • luckily, I can just trigger click in this case, which pretty much is the same operation (since the element is not being moved - this is the selenium 3 fix.

In terms of code

Drag Events before:

• dragstart on source element
• drop on target element
  Drag Events after (which I think should be more realistic):
• dragstart on source element
• dragover on target element
• drop on target element
• dragend on source element

Driver Mouse Events before:

• mouse button down
• if sourceElement !== targetElement mouse move
• mouse button up

Driver Mouse Events after:

• mouse move
• mouse button down
• (dragging starts)
• mouse move
• mouse button up
• (dragging ends)

I also switched to more modern javascript event construction. Not sure if we should worry about old browsers, like say MSIE. Compatibility chart: https://caniuse.com/mdn-api_dragevent_dragevent

Note that sticking to the old code has the opposite problem - it uses deprecated functionality that might break in new browsers - if we absolutely want to keep maximum compatibility, I suppose we could use a fallback ("if createEvent exists use it, else use constructor").

---

Live testing: https://jsfiddle.net/0L6kthxv/1/

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.07%. Comparing base (717c5b3) to head (2ca5e26).
Report is 1 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #408      +/-   ##
============================================
- Coverage     86.21%   86.07%   -0.14%     
+ Complexity      185      184       -1     
============================================
  Files             1        1              
  Lines           515      510       -5     
============================================
- Hits            444      439       -5     
  Misses           71       71              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[aik099]

I also switched to more modern javascript event construction. Not sure if we should worry about old browsers, like say MSIE. Compatibility chart: https://caniuse.com/mdn-api_dragevent_dragevent

Note that sticking to the old code has the opposite problem - it uses deprecated functionality that might break in new browsers - if we absolutely want to keep maximum compatibility, I suppose we could use a fallback ("if createEvent exists use it, else use constructor").

I agree. We'll use the new event invocation code and see how it will perform.

In terms of code ...

Looking at the invoked events (proposed version in this PR) I've noticed that 99.9% matches (except for triggering the mousemove event on items that the cursor ‘passes over’ while moving part) what GitHub Copilot told me when I asked about what JavaScript events are invoked by Selenium Server itself when performing dragAndDrop action using WebDriver Action API.

It also said, that for cases, when drag-and-dropping onto itself same set of events is called but on the same element. This is where an approach with the click event, you've proposed, might result in the incorrect event emulation sequence.

You're proposition definitely looks superior compared with the current (in the master branch) implementation. Since both implementations were firing basic drag-related events, then no tests failed.

P.S.
I'm wondering how you even noticed inconsistency in drag-and-drop event emulation?

I've also updated your PR description (see **// note by @aik099: ... lines). Feel free to apply proposed changes to PR description as needed or remove my notes if I'm mistaken.

[uuf6429]

I'm wondering how you even noticed inconsistency in drag-and-drop event emulation?

While investigating the error, I noticed that we trigger dragstart and drop, but not dragend - I did some research for the rest.

I've also updated your PR description

Thanks, that made sense. 👍

This is where an approach with the click event, you've proposed, might result in the incorrect event emulation sequence.

Either we have to "live with it" (in selenium 3, I could make a condition to keep previous behaviour), OR now that I think about it, maybe we should switch to javascript events for that too? That should also solve your question about coverage. What do you think?
(in that case, we might want to consider how we should handle touch events and/or pointer events)

[uuf6429]

Just for testing, I've added the following code to the js_test.html page:

            function getStandardEventNames(element) {
                const eventNames = [];
                for (const prop in element) {
                    if (!prop.startsWith("on")) {
                        continue;
                    }

                    const eventName = prop.slice(2);
                    try {
                        new Event(eventName);
                        eventNames.push(eventName);
                    } catch {
                        // if an error is thrown, it's not a valid event name
                    }
                }

                return eventNames;
            }

            function monitorStandardEvents(element) {
                for (const eventName of getStandardEventNames(element)) {
                    element.addEventListener(eventName, event => console.log(event.target.id ?? 'unknown', eventName));
                }
            }

            setInterval(() => console.log('-------------------------------------'), 5000);
            // ^-- since we trigger a lot of unrelated events (e.g. while moving over element before or after drag and drop, I use this line to mark the beginning and the end of the manual test.

            monitorStandardEvents(document.getElementById('draggable'));
            monitorStandardEvents(document.getElementById('draggable2'));
            monitorStandardEvents(document.getElementById('droppable'));

            document.getElementById('draggable').setAttribure('draggable', 'true');
            document.getElementById('draggable2').setAttribure('draggable', 'true');

I also had to comment out the $("#draggable")... and $( "#draggable2")... jQuery parts to enable native drag & drop.

After dragging the smaller & darker grey box to the right-most light grey box, I got the following event log:

-------------------------------------
draggable2 pointerdown
draggable2 mousedown
draggable2 pointermove
draggable2 mousemove
draggable2 pointermove  ---.__ Repetition depends on user making slight
draggable2 mousemove    ---'   pauses while moving mouse.
draggable2 dragstart
draggable2 pointercancel
draggable2 pointerout
draggable2 pointerleave
droppable pointerover
droppable pointerenter
draggable2 mouseout
draggable2 mouseleave
droppable mouseover
droppable mouseenter
droppable mousemove
droppable pointerout
droppable pointerleave
droppable mouseout
droppable mouseleave
draggable2 drag
draggable2 dragend
-------------------------------------

That is effectively the sequence of events firefox generates.

[aik099]

I'm wondering how you even noticed inconsistency in drag-and-drop event emulation?

While investigating the error, I noticed that we trigger dragstart and drop, but not dragend - I did some research for the rest.

What error investigation are you talking about? You've mentioned drag-and-drop failure (in PR description), but which one? The only drag-related failure from recent times was with drag-onto-itself test and was caused by an outdated Firefox version.

This is where an approach with the click event, you've proposed, might result in the incorrect event emulation sequence.

Either we have to "live with it" (in selenium 3, I could make a condition to keep previous behaviour), OR now that I think about it, maybe we should switch to javascript events for that too? What do you think? (in that case, we might want to consider how we should handle touch events and/or pointer events)

There are no IFs in the driver code to perform different drag-n-drop behavior based on the Selenum Server version. The inconsistency in the build configuration (see below answer) might have resulted in that assumption.

Please keep improved drag-and-drop behavior consistent (always on) regardless of the used Selenium Server version.

... That should also solve your question about coverage. ...

I've figured out why there was no coverage. The test, that covers it is ignored in Selenium 3+Firefox, but runs on Selenum 2+Firefox. Since I'm testing only on Firefox I got this line uncovered. Probably need to update build config to test Selenium2/3 on Chome at least on 1 PHP version. I've added #409 about this.

[uuf6429]

What error investigation are you talking about? You've mentioned drag-and-drop failure (in PR description), but which one? The only drag-related failure from recent times was with drag-onto-itself test and was caused by an outdated Firefox version.

Right, that's what I meant. Interestingly, I couldn't get the test to pass (without my changes) on any Firefox version.

I checked these two tests:

• \Behat\Mink\Tests\Driver\Js\JavascriptTest::testDragDrop
• \Behat\Mink\Tests\Driver\Js\JavascriptTest::testDragDropOntoHiddenItself

Results:

• selenium/standalone-firefox-debug:3
  Both failed with:

  Failed asserting that two strings are identical.
  Expected :'Dropped left!'
  Actual   :'Drop here'
  

• selenium/standalone-firefox:2
  I get this in all cases:

  Could not open connection: Unable to connect to host 127.0.0.1 on port 7055 after 45000 ms.
  

• selenium/standalone-firefox-debug:2.53.1
  Both failed with:

  Failed asserting that two strings are identical.
  Expected :'Dropped left!'
  Actual   :'Drop here'
  

I have no idea why the behaviour is so different compared to GitHub actions.

There are no IFs in the driver code to perform different drag-n-drop behavior based on the Selenum Server version.

Yes, I know. What I meant was to have a condition for Selenium 3 to avoid the clicking problem, but now I suspect it would work anyway since I got the same results on Selenium 2.

Please keep improved drag-and-drop behavior consistent (always on) regardless of the used Selenium Server version.

In that case, we would need to also trigger the mouse/pointer events from javascript.

[aik099]

... Right, that's what I meant. Interestingly, I couldn't get the test to pass (without my changes) on any Firefox version. ...

For local testing, I launch Selenium Server's JAR file and let it control the browsers, which I have installed locally (Chrome/Firefox). I don't use Docker Selenium images at all. None of the drag-related tests fail in Firefox for me. Maybe they fail only in some Firefox versions due to geckodriver issues and exactly that version is used in the Selenium Docker image.

Debugging the cause of failure you have locally (via Docker) might reveal some interesting bugs.

In that case, we would need to also trigger the mouse/pointer events from javascript.

Sure. Please do so. Hopefully that won't break jQuery drag-and-drop JS used by test suite.

[uuf6429]

@aik099 what worries me now is that triggering mouse/pointer events is not so easy: I have some trouble figuring out the various positions normally available with these events. I suspect that when I trigger such events fields like x/y, clientX/Y, layerX/Y, screenX/Y, pageX/Y are either undefined or zeroed out. 😞

[aik099]

aik099 what worries me now is that triggering mouse/pointer events is not so easy: I have some trouble figuring out the various positions normally available with these events. I suspect that when I trigger such events fields like x/y, clientX/Y, layerX/Y, screenX/Y, pageX/Y are either undefined or zeroed out. 😞

@uuf6429 , are above-mentioned event properties correctly populated when Selenium Server 3 is triggering these JS events internally as part of the Actions API calls?

[uuf6429]

are above-mentioned event properties correctly populated when Selenium Server 3 is triggering these JS events internally as part of the Actions API calls?

@aik As far as I know, Selenium 3 triggers those events through browser API, so I'm pretty sure those fields would be set.

In any case, my last commit avoids that problem and somehow fixes the tests (locally, Se3+Chrome was failing before this PR). It's not entirely clear why...maybe it was more of a problem that the old code didn't trigger the correct sequence of D&D events? 🤔

[aik099]

Could please use long array syntax (array(...)) instead of short array syntax ([...]). I'm not against this in general (especially since PHP 7.2 is required to use this driver), but we need to be consistent.

If you'd like, you can do large scale replace and maybe add phpstan rule to check this.

[uuf6429]

@aik099 small reminder; I don't have write access to this repo.

[aik099]

@aik099 small reminder; I don't have write access to this repo.

@uuf6429 , if you need my assistance with anything (e.g. review the PR or merge it), then I'm here to help. Just let me know.

In this particular case I should have guessed myself that PR review is requested (because I've asked one thing and you've made one commit), but better to be explicit about your expectations to avoid misunderstandings later on.

[aik099]

Merging, thanks @uuf6429 .

[uuf6429]

@aik099 I just realised that the problem mentioned in the description (Failed asserting that two strings are identical. Expected :'Dropped small!' Actual :'Drop here') is actually still happening.

I've figured why and what happened:

1. The problem happens only on Firefox - in particular on Selenium 2.53.1.
2. While working on this PR, at some point I switched to Chrome, and due to some broken code, I got the same failure mentioned above. After fixing the code, the tests started passing - but I didn't realise that I should have switched to Firefox.

In short: this PR improves the D&D event flow, but it doesn't fix the original problem. 😞

At least we now know it's a problem with Firefox. I'll work on a new and separate PR.

[uuf6429]

@aik099 hmm, I've found that we are actually supposed to skip the testDragDropOntoHiddenItself test on firefox. I'll check the driver-test-suite CI, there must be something wrong there that is somehow running the tests anyway. I had the same situation locally, which I just fixed.

You can ignore my last two comments.

Edit: I've figured out why that test was being executed; I missed setting up the SELENIUM_VERSION env var so that test was executed since we skip it only on selenium 2.