w3c / webdriver

Remote control interface that enables introspection and control of user agents.
https://w3c.github.io/webdriver/
Other
679 stars 194 forks source link

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

Open whimboo opened 1 year ago

whimboo commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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/w3c/webdriver/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.