wdspec tests: Check correct node type instead of asserting error for script execution by servo-wpt-sync · Pull Request #55534 · web-platform-tests/wpt

servo-wpt-sync · 2025-10-19T14:22:15Z

These tests suggest that attribute nodes, document etc. are invalid node. That is simply not true based on spec.

Only Firefox pass these tests, while Chrome/Safari fails.
https://wpt.fyi/results/webdriver/tests/classic/execute_script?label=master

There is an ongoing discussion: w3c/webdriver#1687 (comment)

--
EDIT: Instead of removing these tests, we check

whether the node type returned is correct
whether the serialization for enumerable properties for fallback plain object is correct.
Reviewed in wdspec tests: Check correct node type instead of asserting error for script execution servo/servo#39988

wpt-pr-bot

The review process for this patch is being conducted in the Servo project.

whimboo

Very good find. This is actually working in Firefox due to https://bugzilla.mozilla.org/show_bug.cgi?id=1823444 - we are trying to serialize all values and not just those that are enumerable.

I'm fine with updating the tests but as mentioned inline please do not remove them.

whimboo · 2025-10-20T09:01:34Z

webdriver/tests/classic/execute_async_script/node.py

-    (""" document.querySelector("svg").attributes[0] """),
-    (""" document.querySelector("div#text-node").childNodes[1] """),
-    (""" document.querySelector("foo").childNodes[1] """),
-    (""" document.createProcessingInstruction("xml-stylesheet", "href='foo.css'") """),
-    (""" document.querySelector("div#comment").childNodes[0] """),
-    (""" document"""),
-    (""" document.doctype"""),
-], ids=["attribute", "text", "cdata", "processing_instruction", "comment", "document", "doctype"])
-def test_not_supported_nodes(session, inline, expression):


We should not remove all of these tests but move them up to the test right before to check that the correct nodes are returned.

whimboo · 2025-10-20T09:01:59Z

webdriver/tests/classic/execute_async_script/node.py

+def test_queryselector_null_child_access_errors(session, inline):
    session.url = inline(PAGE_DATA)

+    expression = "document.querySelector('foo').childNodes[1]"


We can remove this test given that it hasn't anything to do with DOM nodes.

Looks like you missed to remove this test from this file while it's gone in the other one.

whimboo

Thanks for the update! Just some nits left.

whimboo · 2025-10-21T06:19:21Z

webdriver/tests/classic/execute_async_script/node.py

+def test_node_type(session, inline, expression, expected_type):
+    session.url = inline(PAGE_DATA)
+
+    response = execute_async_script(session, f"arguments[0]({expression}.nodeType)")


To be sure that we correctly serialize those other node types as plain objects (including only with those properties that are enumerable) we have to return the whole serialized object and not only their node type:

Suggested change

response = execute_async_script(session, f"arguments[0]({expression}.nodeType)")

response = execute_async_script(session, f"arguments[0]({expression})")

It seems the returned serialized object won't contain nodeType tho. Do we still want to check node type then?

Mind sharing the payload as returned for one of the cases?

The one for document. Rest is all empty {}.

{'location': {'assign': {}, 'hash': '', 'host': 'web-platform.test:8443', 'hostname': 'web-platform.test', 'href': 'https://web-platform.test:8443/webdriver/tests/support/inline.py?doc=%3C%21doctype+html%3E%0A%3Cmeta+charset%3DUTF-8%3E%0A%0A++++%3Cdiv+id%3D%22deep%22%3E%3Cp%3E%3Cspan%3E%3C%2Fspan%3E%3C%2Fp%3E%3Cbr%2F%3E%3C%2Fdiv%3E%0A++++%3Cdiv+id%3D%22text-node%22%3E%3Cp%3E%3C%2Fp%3ELorem%3C%2Fdiv%3E%0A++++%3Cbr%2F%3E%0A++++%3Csvg+id%3D%22foo%22%3E%3C%2Fsvg%3E%0A++++%3Cdiv+id%3D%22comment%22%3E%3C%21--+Comment+--%3E%3C%2Fdiv%3E%0A++++%3Cscript%3E%0A++++++++var+svg+%3D+document.querySelector%28%22svg%22%29%3B%0A++++++++svg.setAttributeNS%28%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%2C+%22svg%3Afoo%22%2C+%22bar%22%29%3B%0A++++%3C%2Fscript%3E%0A&mime=text%2Fhtml&charset=UTF-8', 'origin': 'https://web-platform.test:8443', 'pathname': '/webdriver/tests/support/inline.py', 'port': '8443', 'protocol': 'https:', 'reload': {}, 'replace': {}, 'search': '?doc=%3C%21doctype+html%3E%0A%3Cmeta+charset%3DUTF-8%3E%0A%0A++++%3Cdiv+id%3D%22deep%22%3E%3Cp%3E%3Cspan%3E%3C%2Fspan%3E%3C%2Fp%3E%3Cbr%2F%3E%3C%2Fdiv%3E%0A++++%3Cdiv+id%3D%22text-node%22%3E%3Cp%3E%3C%2Fp%3ELorem%3C%2Fdiv%3E%0A++++%3Cbr%2F%3E%0A++++%3Csvg+id%3D%22foo%22%3E%3C%2Fsvg%3E%0A++++%3Cdiv+id%3D%22comment%22%3E%3C%21--+Comment+--%3E%3C%2Fdiv%3E%0A++++%3Cscript%3E%0A++++++++var+svg+%3D+document.querySelector%28%22svg%22%29%3B%0A++++++++svg.setAttributeNS%28%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%2C+%22svg%3Afoo%22%2C+%22bar%22%29%3B%0A++++%3C%2Fscript%3E%0A&mime=text%2Fhtml&charset=UTF-8', 'toString': {}}}

What do you mean with rest is all empty? We should be able to serialize attributes etc or do the ones not have any enumerable properties?

Beside the single node that we return here, we could return a dict including the nodeType for a check.

Seems print won't work and not printing anything..

Beside the single node that we return here, we could return a dict including the nodeType for a check.

Are you suggesting below?

response1 = execute_script("return {expression}") assert_success(response1) response2 = execute_script("return {expression}.nodeType") result = assert_success(response2) assert result == NODE_TYPE[expected_type]

Please note that you cannot test this with Firefox due to the bug I mentioned earlier in this PR.

What I meant is:

response = execute_script(f"const result = {expression}; return {{ "result": result, "type": result.nodeType }}") assert_success(response) assert result["type"] == NODE_TYPE[expected_type]

The other ones also have no enumerable properties as it seems. Only document do.

Looks like this is fine and according to the spec. For documents only location seems to be enumerable as returned by Object.keys(document). So lets just test that we can serialize non Element nodes but no specific property beside the separately returned node type. Thanks.

Now we have to invocations for script evaluation. Lets have it instead in a single one - async in this file and non-async in the other. Please note that my example was only for the non-async case and needs to be adjusted in this file.

Yeah I included below but forgot to remove this.

webdriver/tests/classic/execute_async_script/node.py

whimboo · 2025-10-21T06:25:29Z

webdriver/tests/classic/execute_async_script/node.py

+def test_queryselector_null_child_access_errors(session, inline):
    session.url = inline(PAGE_DATA)

+    expression = "document.querySelector('foo').childNodes[1]"


Looks like you missed to remove this test from this file while it's gone in the other one.

Signed-off-by: Euclid Ye <[email protected]>

whimboo · 2025-10-22T06:02:24Z

webdriver/tests/classic/execute_async_script/node.py

+def test_node_type(session, inline, expression, expected_type):
+    session.url = inline(PAGE_DATA)
+
+    response = execute_async_script(session, f"arguments[0]({expression}.nodeType)")


Now we have to invocations for script evaluation. Lets have it instead in a single one - async in this file and non-async in the other. Please note that my example was only for the non-async case and needs to be adjusted in this file.

webdriver/tests/classic/execute_script/node.py

Signed-off-by: Euclid Ye <[email protected]>

whimboo · 2025-10-22T07:00:07Z

webdriver/tests/classic/execute_async_script/node.py

+    response = execute_async_script(
+        session,
+        f"""
+    const result = {expression};


You have to adjust the code here for async scripts. If you run this test locally it should never finish. For async scripts you need to call the resolve method which is passed in as last argument via arguments. See other tests in this folder how it needs to look like.

I used arguments[0] right below, which is the resolve method. This is same as other tests.

Ah, I somehow missed that, sorry. Maybe you can add a const [resolve] = arguments; at the top of the script, and then call resolve()? It makes it more obvious and easier to spot.

whimboo

Looks good to me.

@sadym-chromium can you please cross-check yourself? Those changes will trigger some work for the serialization steps in Webdriver classic.

whimboo · 2025-10-22T07:36:43Z

webdriver/tests/classic/execute_async_script/node.py

+    response = execute_async_script(
+        session,
+        f"""
+    const result = {expression};


Ah, I somehow missed that, sorry. Maybe you can add a const [resolve] = arguments; at the top of the script, and then call resolve()? It makes it more obvious and easier to spot.

Signed-off-by: Euclid <[email protected]>

2. Remove test unrelated to node Signed-off-by: Euclid Ye <[email protected]>

Signed-off-by: Euclid Ye <[email protected]>

…stead of asserting error for script execution, a=testonly Automatic update from web-platform-tests Update outdated test to align spec Signed-off-by: Euclid <[email protected]> -- 1. Test node type instead of removing all 2. Remove test unrelated to node Signed-off-by: Euclid Ye <[email protected]> -- Fix test that never selected anything correctly Signed-off-by: Euclid Ye <[email protected]> -- Cdata test Signed-off-by: Euclid Ye <[email protected]> -- Update async test symetrically Signed-off-by: Euclid Ye <[email protected]> -- Remove unused test and reduce duplication Signed-off-by: Euclid Ye <[email protected]> -- dummy commit to check web-platform-tests/wpt#55534 (comment) Signed-off-by: Euclid Ye <[email protected]> -- Revert some changes and check enumerable serialization Signed-off-by: Euclid Ye <[email protected]> -- Reordeing assertions Signed-off-by: Euclid Ye <[email protected]> -- minor adjustment Signed-off-by: Euclid Ye <[email protected]> -- wpt-commits: a3f865d9fc1ecec0c167fe9d321f8ba0f7fe4508, 8c0c67d078f571dd58d9e230be2324d109efed67, 2db0d3c2dcb52cb8b3f6771120072d6212748833, 65a4074ad4b31ebca34c8dc55a4534799e5d9735, 191c2d8c3dc0f391d150da6fd6f1216e58d95fa6, 05a87ac6c74f0ddbe70db117cda572c4b06b3e02, bdbfb04cb441026fef5bf033c82c50fc9b3246cd, 8c6c3835d5864fce20fa04bd7569151963f572eb, 1cd8a8b225fbdeaa53d858b2e4c727bcaf7abc66, 41f1c2edcddb318eadef475b8a2a20688c7f163f wpt-pr: 55534

…stead of asserting error for script execution, a=testonly Automatic update from web-platform-tests Update outdated test to align spec Signed-off-by: Euclid <yezhizhenjiakanggmail.com> -- 1. Test node type instead of removing all 2. Remove test unrelated to node Signed-off-by: Euclid Ye <yezhizhenjiakanggmail.com> -- Fix test that never selected anything correctly Signed-off-by: Euclid Ye <yezhizhenjiakanggmail.com> -- Cdata test Signed-off-by: Euclid Ye <yezhizhenjiakanggmail.com> -- Update async test symetrically Signed-off-by: Euclid Ye <yezhizhenjiakanggmail.com> -- Remove unused test and reduce duplication Signed-off-by: Euclid Ye <yezhizhenjiakanggmail.com> -- dummy commit to check web-platform-tests/wpt#55534 (comment) Signed-off-by: Euclid Ye <yezhizhenjiakanggmail.com> -- Revert some changes and check enumerable serialization Signed-off-by: Euclid Ye <yezhizhenjiakanggmail.com> -- Reordeing assertions Signed-off-by: Euclid Ye <yezhizhenjiakanggmail.com> -- minor adjustment Signed-off-by: Euclid Ye <yezhizhenjiakanggmail.com> -- wpt-commits: a3f865d9fc1ecec0c167fe9d321f8ba0f7fe4508, 8c0c67d078f571dd58d9e230be2324d109efed67, 2db0d3c2dcb52cb8b3f6771120072d6212748833, 65a4074ad4b31ebca34c8dc55a4534799e5d9735, 191c2d8c3dc0f391d150da6fd6f1216e58d95fa6, 05a87ac6c74f0ddbe70db117cda572c4b06b3e02, bdbfb04cb441026fef5bf033c82c50fc9b3246cd, 8c6c3835d5864fce20fa04bd7569151963f572eb, 1cd8a8b225fbdeaa53d858b2e4c727bcaf7abc66, 41f1c2edcddb318eadef475b8a2a20688c7f163f wpt-pr: 55534 UltraBlame original commit: 1ce4adbda47b6f567c6e500538b4ad7cc301cded

servo-wpt-sync added do not merge yet servo-export labels Oct 19, 2025

servo-wpt-sync mentioned this pull request Oct 19, 2025

wdspec tests: Check correct node type instead of asserting error for script execution servo/servo#39988

Merged

wpt-pr-bot added webdriver wg-s_browser-tools-testing labels Oct 19, 2025

wpt-pr-bot approved these changes Oct 19, 2025

View reviewed changes

servo-wpt-sync changed the title ~~webdriver: Remove outdated script execution tests to align spec~~ webdriver: Remove outdated script execution tests to align with spec Oct 19, 2025

servo-wpt-sync force-pushed the servo_export_39988 branch 2 times, most recently from e08f940 to f2b6b6f Compare October 20, 2025 01:08

whimboo requested changes Oct 20, 2025

View reviewed changes

whimboo mentioned this pull request Oct 20, 2025

Should HTMLDocument nodes be serialized as web element reference? w3c/webdriver#1687

Closed

servo-wpt-sync force-pushed the servo_export_39988 branch from f2b6b6f to 16f971b Compare October 21, 2025 03:40

servo-wpt-sync changed the title ~~webdriver: Remove outdated script execution tests to align with spec~~ webdriver: Check correct node type instead of asserting error Oct 21, 2025

servo-wpt-sync changed the title ~~webdriver: Check correct node type instead of asserting error~~ wdspec tests: Check correct node type instead of asserting error Oct 21, 2025

servo-wpt-sync force-pushed the servo_export_39988 branch from 16f971b to ab2b7d3 Compare October 21, 2025 05:49

servo-wpt-sync changed the title ~~wdspec tests: Check correct node type instead of asserting error~~ wdspec tests: Check correct node type instead of asserting error for script execution Oct 21, 2025

whimboo requested changes Oct 21, 2025

View reviewed changes

servo-wpt-sync force-pushed the servo_export_39988 branch 3 times, most recently from 88b66c2 to 8b7617d Compare October 21, 2025 07:51

yezhizhen added a commit to servo/servo that referenced this pull request Oct 21, 2025

dummy commit to check web-platform-tests/wpt#55534 (comment)

18ea840

Signed-off-by: Euclid Ye <[email protected]>

servo-wpt-sync pushed a commit to servo/wpt that referenced this pull request Oct 21, 2025

dummy commit to check web-platform-tests#55534 (comment)

d2e113a

Signed-off-by: Euclid Ye <[email protected]>

servo-wpt-sync force-pushed the servo_export_39988 branch from 8b7617d to d2e113a Compare October 21, 2025 09:01

servo-wpt-sync pushed a commit to servo/wpt that referenced this pull request Oct 22, 2025

dummy commit to check web-platform-tests#55534 (comment)

51d58a3

Signed-off-by: Euclid Ye <[email protected]>

servo-wpt-sync force-pushed the servo_export_39988 branch from d2e113a to 66d75e9 Compare October 22, 2025 04:01

whimboo requested changes Oct 22, 2025

View reviewed changes

servo-wpt-sync pushed a commit to servo/wpt that referenced this pull request Oct 22, 2025

dummy commit to check web-platform-tests#55534 (comment)

d0e13d1

Signed-off-by: Euclid Ye <[email protected]>

servo-wpt-sync force-pushed the servo_export_39988 branch from 66d75e9 to 0149dfd Compare October 22, 2025 06:33

whimboo requested changes Oct 22, 2025

View reviewed changes

whimboo approved these changes Oct 22, 2025

View reviewed changes

whimboo requested a review from sadym-chromium October 22, 2025 07:40

yezhizhen added 10 commits October 22, 2025 12:05

Update outdated test to align spec

5c405c1

Signed-off-by: Euclid <[email protected]>

1. Test node type instead of removing all

f8b1208

2. Remove test unrelated to node Signed-off-by: Euclid Ye <[email protected]>

Fix test that never selected anything correctly

5de1765

Signed-off-by: Euclid Ye <[email protected]>

Cdata test

c0a11a2

Signed-off-by: Euclid Ye <[email protected]>

Update async test symetrically

6ff179f

Signed-off-by: Euclid Ye <[email protected]>

Remove unused test and reduce duplication

9b940f8

Signed-off-by: Euclid Ye <[email protected]>

dummy commit to check web-platform-tests#55534 (comment)

6aa9785

Signed-off-by: Euclid Ye <[email protected]>

Revert some changes and check enumerable serialization

9b456e2

Signed-off-by: Euclid Ye <[email protected]>

Reordeing assertions

4bbea97

Signed-off-by: Euclid Ye <[email protected]>

minor adjustment

a53aae3

Signed-off-by: Euclid Ye <[email protected]>

servo-wpt-sync force-pushed the servo_export_39988 branch from 0149dfd to a53aae3 Compare October 22, 2025 12:07

servo-wpt-sync removed the do not merge yet label Oct 23, 2025

servo-wpt-sync merged commit 41f1c2e into web-platform-tests:master Oct 23, 2025
25 checks passed

servo-wpt-sync pushed a commit that referenced this pull request Oct 23, 2025

dummy commit to check #55534 (comment)

bdbfb04

Signed-off-by: Euclid Ye <[email protected]>

servo-wpt-sync deleted the servo_export_39988 branch October 23, 2025 03:28

mertcanaltin pushed a commit to mertcanaltin/wpt that referenced this pull request Oct 26, 2025

dummy commit to check web-platform-tests#55534 (comment)

1cccf56

Signed-off-by: Euclid Ye <[email protected]>

	response = execute_async_script(session, f"arguments[0]({expression}.nodeType)")
	response = execute_async_script(session, f"arguments[0]({expression})")

Conversation

servo-wpt-sync commented Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpt-pr-bot left a comment

Choose a reason for hiding this comment

Uh oh!

whimboo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whimboo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yezhizhen Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whimboo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

servo-wpt-sync commented Oct 19, 2025 •

edited

Loading

yezhizhen Oct 21, 2025 •

edited

Loading