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

Wrong events for modifier-click (Actions API) #888

Closed
codingphil opened this issue Aug 21, 2017 · 6 comments
Closed

Wrong events for modifier-click (Actions API) #888

codingphil opened this issue Aug 21, 2017 · 6 comments
Labels

Comments

@codingphil
Copy link

System

  • Version: 0.18
  • Platform: Windows 10 Creators Update
  • Firefox: 55.0.2
  • Selenium: 3.4

Testcase

The following test case executes CONTROL-Click on a page which reports the triggered JS events.
The ctrlKey property of the click event (and others) is expected to be set but actually is not.

This test uses the same test page regression test of this Mozilla bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1367430

  private Map getEvent(ArrayList<Map> events, String eventName) {
    ArrayList<Map> filteredEvents = events.stream().filter(e -> e.get("type").equals(eventName))
        .collect(Collectors.toCollection(ArrayList::new));
    assertEquals(String.format("event '%s' was not triggered once", eventName), 1l, filteredEvents.size());
    return filteredEvents.get(0);
  }

  @Test
  public void testCtrlClick_keyDown_clickOnElem_keyUp() {
    driver.navigate().to(TestPages.MOZILLA_TEST_ACTIONS_WDSPEC);
    WebElement clickTarget = driver.findElementById("outer");

    Keys modifier = Keys.CONTROL;
    new Actions(driver)
      .keyDown(modifier)
      .click(clickTarget)
      .keyUp(modifier)
      .perform();

    ArrayList<Map> events = (ArrayList<Map>) driver.executeScript("return allEvents.events;");

    Map clickEvent = getEvent(events, "click");
    assertEquals("CTRL key not pressed during 'click'", true, clickEvent.get("ctrlKey")); // FAILS!

    Map mouseDownEvent = getEvent(events, "mousedown");
    assertEquals("CTRL key not pressed during 'mousedown'", true, mouseDownEvent.get("ctrlKey"));

    Map mouseUpEvent = getEvent(events, "mouseup");
    assertEquals("CTRL key not pressed during 'mouseup'", true, mouseUpEvent.get("ctrlKey"));
  }

Test HTML page:
test_actions_wdspec.zip

Stacktrace

java.lang.AssertionError: CTRL key not pressed during 'click' expected:<true> but was:<false>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:118)
	at ModifierClickTests.testCtrlClick_moveToElem_keyDown_click_keyUp(ModifierClickTests.java:98)

Trace-level log

geckodriver.zip

@andreastt
Copy link
Contributor

@mjzffr Wasn’t https://bugzilla.mozilla.org/show_bug.cgi?id=1367430 supposed to have fixed this, or did that only patch key modifiers for input to keyboard devices?

@mjzffr
Copy link
Contributor

mjzffr commented Aug 21, 2017

I think this is a problem with how the Selenium client constructs the payload for the actions endpoint. At the least, it's misleading to users of the API.

The attached geckodriver.log shows that we receive the following:

{"actions": [{"actions": [{"duration": 0, "type": "pause"},
                          {"duration": 100,
                           "origin": {"ELEMENT": "5d3e3dd0-9260-4e07-8553-2c1bb1e22ac9",
                                      "element-6066-11e4-a52e-4f735466cecf": "5d3e3dd0-9260-4e07-8553-2c1bb1e22ac9"},
                           "type": "pointerMove",
                           "x": 0,
                           "y": 0},
                          {"button": 0, "type": "pointerDown"},
                          {"button": 0, "type": "pointerUp"}],
              "id": "default mouse",
              "parameters": {"pointerType": "mouse"},
              "type": "pointer"},
             {"actions": [{"type": "keyDown", "value": "\xee\x80\x89"},
                          {"type": "keyUp", "value": "\xee\x80\x89"}],
              "id": "default keyboard",
              "type": "key"}]}

Marionette follows the spec and transposes this to actions-by-tick such that the first tick consists of:

  • {"duration": 0, "type": "pause"}
  • {"type": "keyDown", "value": "\xee\x80\x89"}

The second tick is:

  • the pointerMove with duration 100
  • {"type": "keyUp", "value": "\xee\x80\x89"}

The third tick is:

  • {"button": 0, "type": "pointerDown"}

The fourth tick is:

  • {"button": 0, "type": "pointerUp"}

So the keyUp action is already complete by the time we get to the pointerDown.

@codingphil
Copy link
Author

codingphil commented Aug 22, 2017

As I understand it this is a bug in the Selenium Java Binding.

The Actions class does not insert the Pause interactions as required.

https://github.com/SeleniumHQ/selenium/blame/7b81ced5464799587fc582fb5e5bb8560098fab2/java/client/src/org/openqa/selenium/interactions/Actions.java#L553

The collection seenSources is always empty. Initializing this collection (on construction) with all active input sources would fix the problem.

Should we file an issue for selenium?

@andreastt
Copy link
Contributor

Please do file an issue against Selenium. I will keep this open for tracking, and thanks for reporting it! It will be good to track all the action sequence related issues down.

@codingphil
Copy link
Author

Fixed in Selenium 3.5.3

@lock
Copy link

lock bot commented Aug 17, 2019

This issue has been automatically locked since there has not been any recent activity after it was closed. If you have run into an issue you think is related, please open a new issue.

@lock lock bot locked and limited conversation to collaborators Aug 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants