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

Diverging driver behavior for "clear the modifier key state" in "Dispatch actions for a string" #1734

Open
whimboo opened this issue Apr 12, 2023 · 5 comments

Comments

@whimboo
Copy link
Contributor

whimboo commented Apr 12, 2023

As per the Dispatch actions for a string definition in the WebDriver classic spec the modifier key state should only be reset when the NULL key is send. But drivers behave differently these days for the following Selenium test:

def test_modifier_keys(driver):
    driver.get(inline('<input id="input" type="text" value="foo">'))
    elem = driver.find_element(By.ID, "input")
    elem.send_keys(Keys.META + 'a' + Keys.META + Keys.DELETE + "cheese")
    assert elem.get_property("value") == 'cheese'

Per definition the META key should be kept in pressed state and as such cheese should not be the final value of the send_keys command. But here the results of the browsers:

  1. Chrome passes the above assertion, which means the META modifier key has been reset.

  2. Firefox fails this assertion with foo != cheese, which is the cause of a regression in Firefox 99, which we are going to fix for now to have a similar behavior as Chrome.

  3. I wasn't able to test Safari.

We should discuss what the correct behavior should be and then align the WebDriver implementations on the spec. If we think that we should not do such a change, maybe we should add that using the modifier key again resets the state of that particular modifier.

CC @jgraham, @sadym-chromium.

@whimboo
Copy link
Contributor Author

whimboo commented May 3, 2023

We have tested our Mozilla specific test, which we have created for this particular problem, with Safari and it fails there as well. That means we have a different behavior between Firefox and Chrome, and Safari.

So we should decide if:

  1. Changing the spec to allow resetting the modifier state when sending the same modifier again within the same string.
  2. Changing the behavior of Firefox and Chrome to behave like the spec instructs.

@juliandescottes
Copy link
Contributor

Some additional details for the behavior on Safari, with variations on the test mentioned by @whimboo above.

The idea is to do ctrl or cmd + a, then cancel the modifier (either with null or by repeating the key or by splitting the sequence) and then type cheese. I tried four different approaches on Safari, and only one worked. See below.

def test_modifier_key_toggles(session, inline, modifier_key):
    session.url = inline("<input type=text value=foo>")
    element = session.find.css("input", all=False)

    # Fails, value is '' at the end
    # element_send_keys(session, element, f"{modifier_key}a{Keys.NULL}{Keys.DELETE}cheese")

    # Fails, value is '' at the end
    # element_send_keys(session, element, f"{modifier_key}a{Keys.NULL}{Keys.DELETE}")
    # element_send_keys(session, element, "cheese")

    # Fails, value is '' at the end
    # element_send_keys(session, element, f"{modifier_key}a{Keys.NULL}")
    # element_send_keys(session, element, f"{Keys.DELETE}cheese")

    # This one works....
    element_send_keys(session, element, f"{modifier_key}a{Keys.NULL}")
    element_send_keys(session, element, Keys.DELETE)
    element_send_keys(session, element, "cheese")

    assert element.property("value") == "cheese"

@whimboo
Copy link
Contributor Author

whimboo commented May 9, 2023

I've run the individual tests from @juliandescottes comment with Chrome and it works in all the scenarios. Also the following line is passing in Chrome:

element_send_keys(session, element, f"{modifier_key}a{modifier_key}{Keys.DELETE}cheese")

It means that it is also not WebDriver spec compliant similar to Firefox with the recent fix for the regression (bug 1776190). Sadly I'm still not able to run the wpt tests with Safari, so I cannot test the above extra test.

Overall we may want to update the WebDriver classic spec to allow resetting the modifier key if it gets pressed again within the dispatch actions for a string processing. That would probably (unsure in regards of Safari) not cause backward incompatibility.

@juliandescottes
Copy link
Contributor

Safari also doesn't support sending the modifier key again at all. Replacing NULL by modifier_key in the test above fails for all 4 scenarios.

def test_modifier_key_toggles(session, inline, modifier_key):
    session.url = inline("<input type=text value=foo>")
    element = session.find.css("input", all=False)

    # Fails, value is 'foo' at the end
    # element_send_keys(session, element, f"{modifier_key}a{modifier_key}{Keys.DELETE}cheese")

    # Fails, value is 'foo' at the end
    # element_send_keys(session, element, f"{modifier_key}a{modifier_key}{Keys.DELETE}")
    # element_send_keys(session, element, "cheese")

    # Fails, value is 'foo' at the end
    # element_send_keys(session, element, f"{modifier_key}a{modifier_key}")
    # element_send_keys(session, element, f"{Keys.DELETE}cheese")

    # Fails, value is 'cheesefoo' at the end
    # element_send_keys(session, element, f"{modifier_key}a{modifier_key}")
    # element_send_keys(session, element, Keys.DELETE)
    # element_send_keys(session, element, "cheese")

    assert element.property("value") == "cheese"

@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed Diverging driver behavior for "clear the modifier key state" in "Dispatch actions for a string".

The full IRC log of that discussion <jgraham_> Topic: Diverging driver behavior for "clear the modifier key state" in "Dispatch actions for a string"
<jgraham_> GitHub: https://github.com//issues/1734
<jgraham_> whimboo: This is WebDriver classic. We had a bug report / regression on this behaviour. It affects Send Keys; there's a difference between setting an unsetting modifiers. In the spec only sending NULL as a key unsets the modifier. But in Firefox/Chrome sending the same modifier again unsets the modifier. This isn't supported in SafariDriver. This is an interop problem. Can we standardise on the
<jgraham_> Chrome/Firefox behaviour? That allows unsetting a specific modifier at a time.
<jgraham_> q?
<jgraham_> ack mathiasbynens
<mathiasbynens> yes
<patrickangle_> q+
<jgraham_> acl patrickangle_
<jgraham_> ack patrickangle_
<jgraham_> patrickangle_: This feels like a bug on our end; it seems reasonable this should work.
<jgraham_> whimboo: I suspect this behaviour predates the spec.
<jgraham_> patrickangle_: The handling of NULL is in the spec?
<jgraham_> whimboo: Yes, NULL is in the spec, but resending the modifier key to unset it isn't in the spec. There's a workaround (see issue)
<jgraham_> jgraham: I suggest we propose the spec change here and if there are concerns we can discuss again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants