Amend test_document_element_is_interactable #17769

julianrkung · 2019-07-10T18:18:59Z

ChromeDriver was failing this test despite being W3C compliant in this regard. The reason Chrome's webdriver failed was due to Chrome's HTMLDocument not being focusable. In contrast, Firefox's webdriver passes this test because Firefox's HTMLDocument is focusable. According to the W3 spec (linked below), HTMLDocument need not be focusable (can be seen through the absence of the focus method). Therefore this test should be removed since it does not test spec-compliance.

W3 spec:
https://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-1006298752

andreastt

The absence of a focus() method on the Document interface does not mean it doesn’t receive input events.

When WebDriver uses Element Send Keys on the root document element (<body> in this case because the document is HTML), the intention is to mimick the behaviour a user would expect if they type into a freshly loaded document where no element has yet been targetted. This mirrors the behaviour outlined in the UI Events standard except WebDriver doesn’t have an explicit notion the default target.

If we look at what UI Events as to say, it is pretty clear that keydown must be fired regardless of the circumstances, but that its target varies depending on the focussed element and the document type:

Event.target: focused element processing the key event or if no element focused, then the body element if available, otherwise the root element

The HTML document included by the test also works similarly to me in both Chrome and Firefox, so I think this is a bug in chromedriver.

julianrkung · 2019-07-11T17:03:28Z

Deleting the test entirely may have been too rash.

The main problem I have with the test is the following line:
assert session.active_element == element
where element refers to the root element (both Firefox and Chrome have this being the HTML document here).
session.active_element should return the element that currently has focus. In Chrome, since the root element (HTML) is not focusable, the active element is the body, so the check is failed.

Instead I propose the following solution (as it seems this assert is trying to ensure the input is not focused):
assert session.active_element == element or session.active_element == body
as the above would comply with the behavior of Event.target you cited in your post.

I uploaded a patchset with the change and will change the title of the pull request accordingly

andreastt · 2019-07-11T17:37:30Z

Thanks for clarifying exactly why this test fails in Chrome.

I’m somewhat surprised that assert session.active_element == element, where element is the CSS :root element, <html>, passes in Firefox. Similarly to Chrome, document.activeElement returns the expected <body> in Freifox, and I would expect Get Active Element to also do that:

Let active element be the active element of the current browsing context’s document element.

I don’t have time to check the behaviour of Firefox tonight, but can you double-check that session.active_element is returning <body> in Chrome? If that’s not the case, the bug here is actually with Get Active Element, and not with this test assertion.

julianrkung · 2019-07-11T17:59:18Z

Yeah sure confirmed that session.active_element returns in Chrome.

If you're interested here are some logs:
│ > assert session.active_element == element
│ E assert <Element eaef39fb-b9c3-4562-8d45-42b62feef4c3> == <Element ff3797a8-94da-4a4c-9c7f-48257455f0f0>
│ E + where <Element eaef39fb-b9c3-4562-8d45-42b62feef4c3> = <Session 52e1ffe6d3e63dcdaa3eab33961511e4>.active_element
│ body = <Element eaef39fb-b9c3-4562-8d45-42b62feef4c3>
│ element = <Element ff3797a8-94da-4a4c-9c7f-48257455f0f0>
│ response = <Responsetatus=200 body={"value": null}>
│ result = <Element 288c64d9-fa4f-497a-b2a4-943106621f90>
│ session = <Session 52e1ffe6d3e63dcdaa3eab33961511e4>

The body id matches with the session.active_element id

julianrkung · 2019-07-19T18:50:55Z

Any update for this? Do you need any more information for this to be pushed through?

webdriver/tests/element_send_keys/interactability.py

julianrkung · 2019-08-01T14:39:19Z

Any updates on this? I made the change suggested by @whimboo

whimboo · 2019-08-02T11:13:57Z

I'm actually confused by those lines:

│ > assert session.active_element == element
│ E assert <Element eaef39fb-b9c3-4562-8d45-42b62feef4c3> == <Element ff3797a8-94da-4a4c-9c7f-48257455f0f0>
│ E + where <Element eaef39fb-b9c3-4562-8d45-42b62feef4c3> = <Session 52e1ffe6d3e63dcdaa3eab33961511e4>.active_element
│ body = <Element eaef39fb-b9c3-4562-8d45-42b62feef4c3>

Is that the output from the test? Then element has to be the body given that in line 20 we retrieve that via find.css(). But in your case it is the active_element? What is element then? Comparing to the test it doesn't make any sense.

julianrkung · 2019-08-09T19:44:12Z

Sorry for the late response. I'll explain the above log.

session.active_element is <Element eaef39fb-b9c3-4562-8d45-42b62feef4c3> which corresponds to the body element.
element is <Element ff3797a8-94da-4a4c-9c7f-48257455f0f0> which corresponds to the element that is returned from find.css(":root"). The find actually returns <html>, not <body>, in both Firefox and Chrome.

In Chrome, <html> can not be an active element whereas in Firefox <html> can be an active element. This causes Chrome to not be able to pass the test because the test expects the active element to be <html> which is impossible on Chrome. Let me know if anything is unclear sorry for the confusing logs.

JohnChen0 · 2019-10-23T06:30:43Z

@andreastt This PR is blocked by a requested change from you, though from the conversation it isn't clear to me what additional change is needed. I hope the comments from @julianrkung has explained this PR, but please let me know if you any additional questions or concerns. (@julianrkung is a summer intern who has returned to school, and is no longer actively working on ChromeDriver. I'm trying to help pushing this PR through.)

andreastt · 2019-10-23T11:37:22Z

Thanks for reminding me to follow up on this.

<html> can indeed be the active element in Firefox, demonstrated by $("html").focus(). This means Element Send Keys against <html> will make the active element <body> in Chrome, but <html> in Firefox.

The test (without this patch applied) assumes the active element is the CSS root element, which in both browsers is <html>, whereas reality shows the active element is always <body> by default.

I think the change required here is that we should have two tests: one for testing interaction with the document element (<html>) and another for testing interaction with the default active element in a blank page (<body>).

The result of these two tests should be different: in the test for interacting with the document element, sending keys to <html> should be allowed to let the active element assertion be either <body> or <html> as this PR suggests.

Generally we should avoid the use of CSS selectors in these tests as it only adds another layer of indirection for figuring out what the test is meant to test.

I’m somewhat suspecting the difference in browser behaviour could be an interoperability bug, but for the moment we should allow for both behaviours as they are user-observable.

JohnChen0 · 2019-10-23T20:07:24Z

Thanks for the clarification. There is actually already a test that verifies interaction with <body> element, test_body_is_interactable in the same file. Does that suffice for addressing your concerns?

jgraham · 2019-10-25T11:03:29Z

@andreastt I moved your pr/17769 branch to pr_17769 because the former tickles a (silly) bug in our sync. I hope that doesn't cause any problems.

whimboo · 2023-02-27T20:23:46Z

@jgraham we are running only a few tests on this PR. I assume this is happening because we didn't rebase for quite a while? I assume that should be done?

whimboo · 2023-03-10T10:50:26Z

I'm not able to push to the branch used as source here. As such I've created #38930.

Removed test test_document_element_is_interactable

6d288b2

wpt-pr-bot added webdriver wg-testing_browser labels Jul 10, 2019

wpt-pr-bot assigned mjzffr Jul 10, 2019

wpt-pr-bot requested review from andreastt, AutomatedTester, mjzffr, shs96c and whimboo July 10, 2019 18:19

andreastt requested changes Jul 11, 2019

View reviewed changes

Restored test and amended assert to include body

1065467

julianrkung changed the title ~~Remove test test_document_element_is_interactable~~ Amend test_document_element_is_interactable Jul 11, 2019

whimboo reviewed Jul 23, 2019

View reviewed changes

webdriver/tests/element_send_keys/interactability.py Outdated Show resolved Hide resolved

Made updates from reviewer

605746b

mjzffr removed their request for review August 27, 2019 15:19

mjzffr removed their assignment Aug 27, 2019

wpt-pr-bot assigned julianrkung Oct 23, 2019

jgraham closed this Feb 24, 2023

jgraham reopened this Feb 24, 2023

whimboo mentioned this pull request Mar 10, 2023

Updated test_document_element_is_interactable for active element check #38930

Open

whimboo closed this Mar 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Amend test_document_element_is_interactable #17769

Amend test_document_element_is_interactable #17769

julianrkung commented Jul 10, 2019

andreastt left a comment

julianrkung commented Jul 11, 2019

andreastt commented Jul 11, 2019

julianrkung commented Jul 11, 2019

julianrkung commented Jul 19, 2019

julianrkung commented Aug 1, 2019

whimboo commented Aug 2, 2019

julianrkung commented Aug 9, 2019

JohnChen0 commented Oct 23, 2019

andreastt commented Oct 23, 2019

JohnChen0 commented Oct 23, 2019

jgraham commented Oct 25, 2019

whimboo commented Feb 27, 2023

whimboo commented Mar 10, 2023

Amend test_document_element_is_interactable #17769

Amend test_document_element_is_interactable #17769

Conversation

julianrkung commented Jul 10, 2019

andreastt left a comment

Choose a reason for hiding this comment

julianrkung commented Jul 11, 2019

andreastt commented Jul 11, 2019

julianrkung commented Jul 11, 2019

julianrkung commented Jul 19, 2019

julianrkung commented Aug 1, 2019

whimboo commented Aug 2, 2019

julianrkung commented Aug 9, 2019

JohnChen0 commented Oct 23, 2019

andreastt commented Oct 23, 2019

JohnChen0 commented Oct 23, 2019

jgraham commented Oct 25, 2019

whimboo commented Feb 27, 2023

whimboo commented Mar 10, 2023