[py][bidi]: add BiDi script module commands

[navin772]

User description

🔗 Related Issues

💥 What does this PR do?

Adds the low level API commands for the Script module - https://w3c.github.io/webdriver-bidi/#module-script

🔧 Implementation Notes

I have only implemented the commonly used types like EvaluateResult, RealmInfo, RealmType, Source and ResultOwnership. Other types are very nested and not required to be implemented explicitly, we can directly pass them as dict.

💡 Additional Considerations

This PR adds the low level APIs only, I will open another PR that adds the remaining High level BiDi script APIs:

1. pin()
2. unpin()
3. execute()
4. addDomMutationHandler()
5. removeDomMutationHandler()

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)

---

PR Type

Enhancement, Tests

---

Description

• Add BiDi script module commands to Python bindings

  • Implement low-level API for script module (evaluate, callFunction, disown, etc.)
  • Add support for preload scripts, realms, and result ownership
  • Provide dataclasses for script-related types (RealmInfo, EvaluateResult, etc.)

• Add comprehensive tests for BiDi script module

  • Test all new script commands and options (preload, disown, serialization, user activation, etc.)
  • Validate error handling and edge cases for script APIs

---

Changes walkthrough 📝

	Relevant files
Enhancement	script.py Implement BiDi script module commands and types                    py/selenium/webdriver/common/bidi/script.py Implement BiDi script module commands (evaluate, callFunction, disown, etc.) Add dataclasses for script-related types (RealmInfo, EvaluateResult, Source, etc.) Support preload scripts, realms, result ownership, and serialization options Add error handling for invalid argument combinations +374/-0
Tests	bidi_script_tests.py Add comprehensive tests for BiDi script module                      py/test/selenium/webdriver/common/bidi_script_tests.py Add extensive tests for BiDi script module commands and options Test preload scripts, evaluate/callFunction, disown, realms, serialization, user activation Validate error handling and edge cases for script APIs Add utility for shadow root detection in serialization tests +434/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review[bot]]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌ 1234 - Not compliant Non-compliant requirements: • Fix issue where JavaScript in link's href is not triggered on click() in Selenium 2.48 • Ensure compatibility with Firefox 42.0 • Restore functionality that worked in version 2.47.1 5678 - Not compliant Non-compliant requirements: • Fix ChromeDriver ConnectFailure error on Ubuntu 16.04.4 • Resolve connection refused errors for subsequent ChromeDriver instances • Ensure first ChromeDriver instance works without console errors

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Parameter Validation The validation in add_preload_script only checks for both contexts and user_contexts being not None, but doesn't validate if they are empty lists. Empty lists should probably be treated as None for consistency. if contexts is not None and user_contexts is not None: raise ValueError("Cannot specify both contexts and user_contexts") Test Coverage The disown test expects an exception when using disowned handles but catches any Exception. This should be more specific to catch the actual expected exception type from the WebDriver BiDi specification. with pytest.raises(Exception): driver.script.call_function( "function(obj) { return obj.foo; }", await_promise=False, target={"context": driver.current_window_handle}, arguments=[{"handle": handle}], )

[qodo-code-review[bot]]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Validate required JSON fields Suggestion Impact: The suggestion was implemented with improvements. The commit added validation for required fields across multiple classes (RealmInfo, Source, EvaluateResult, ScriptMessage, RealmDestroyed) using individual field checks with specific error messages, and changed from json.get() to direct dictionary access for required fields. code diff: + if "realm" not in json: + raise ValueError("Missing required field 'realm' in RealmInfo") + if "origin" not in json: + raise ValueError("Missing required field 'origin' in RealmInfo") + if "type" not in json: + raise ValueError("Missing required field 'type' in RealmInfo") + return cls( - realm=json.get("realm"), - origin=json.get("origin"), - type=json.get("type"), + realm=json["realm"], + origin=json["origin"], + type=json["type"], context=json.get("context"), sandbox=json.get("sandbox"), ) @@ -83,7 +90,7 @@ context: Optional[str] = None @classmethod - def from_json(cls, json: dict) -> "Source": + def from_json(cls, json: Dict[str, Any]) -> "Source": """Creates a Source instance from a dictionary. Parameters: @@ -94,8 +101,11 @@ ------- Source: A new instance of Source. """ + if "realm" not in json: + raise ValueError("Missing required field 'realm' in Source") + return cls( - realm=json.get("realm"), + realm=json["realm"], context=json.get("context"), ) @@ -110,7 +120,7 @@ exception_details: Optional[dict] = None @classmethod - def from_json(cls, json: dict) -> "EvaluateResult": + def from_json(cls, json: Dict[str, Any]) -> "EvaluateResult": """Creates an EvaluateResult instance from a dictionary. Parameters: @@ -121,9 +131,14 @@ ------- EvaluateResult: A new instance of EvaluateResult. """ + if "realm" not in json: + raise ValueError("Missing required field 'realm' in EvaluateResult") + if "type" not in json: + raise ValueError("Missing required field 'type' in EvaluateResult") + return cls( - type=json.get("type"), - realm=json.get("realm"), + type=json["type"], + realm=json["realm"], result=json.get("result"), exception_details=json.get("exceptionDetails"), ) @@ -140,7 +155,7 @@ self.source = source @classmethod - def from_json(cls, json: dict) -> "ScriptMessage": + def from_json(cls, json: Dict[str, Any]) -> "ScriptMessage": """Creates a ScriptMessage instance from a dictionary. Parameters: @@ -151,10 +166,17 @@ ------- ScriptMessage: A new instance of ScriptMessage. """ + if "channel" not in json: + raise ValueError("Missing required field 'channel' in ScriptMessage") + if "data" not in json: + raise ValueError("Missing required field 'data' in ScriptMessage") + if "source" not in json: + raise ValueError("Missing required field 'source' in ScriptMessage") + return cls( - channel=json.get("channel"), - data=json.get("data"), - source=Source.from_json(json.get("source", {})), + channel=json["channel"], + data=json["data"], + source=Source.from_json(json["source"]), ) @@ -167,7 +189,7 @@ self.realm_info = realm_info @classmethod - def from_json(cls, json: dict) -> "RealmCreated": + def from_json(cls, json: Dict[str, Any]) -> "RealmCreated": """Creates a RealmCreated instance from a dictionary. Parameters: @@ -190,7 +212,7 @@ self.realm = realm @classmethod - def from_json(cls, json: dict) -> "RealmDestroyed": + def from_json(cls, json: Dict[str, Any]) -> "RealmDestroyed": """Creates a RealmDestroyed instance from a dictionary. Parameters: @@ -201,7 +223,10 @@ ------- RealmDestroyed: A new instance of RealmDestroyed. """ - return cls(realm=json.get("realm")) + if "realm" not in json: + raise ValueError("Missing required field 'realm' in RealmDestroyed") + + return cls(realm=json["realm"]) The from_json method should validate that required fields (realm, origin, type) are present in the JSON data. Currently, it allows None values for required fields which could cause issues downstream. py/selenium/webdriver/common/bidi/script.py [57-75] @classmethod def from_json(cls, json: dict) -> "RealmInfo": """Creates a RealmInfo instance from a dictionary. Parameters: ----------- json: A dictionary containing the realm information. Returns: ------- RealmInfo: A new instance of RealmInfo. """ + if not json.get("realm") or not json.get("origin") or not json.get("type"): + raise ValueError("Required fields 'realm', 'origin', and 'type' must be present") + return cls( realm=json.get("realm"), origin=json.get("origin"), type=json.get("type"), context=json.get("context"), sandbox=json.get("sandbox"), ) [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the from_json method for RealmInfo does not validate the presence of required fields (realm, origin, type). The RealmInfo dataclass defines these as non-optional strings, so allowing None values from json.get() could lead to downstream errors. Adding validation improves robustness by ensuring the data conforms to the expected structure.	Medium
[ ] Update

[navin772]

@shbenzer thanks for the review! I have one doubt, currently the low level APIs implemented in this PR are available via the driver instance like - driver.script.add_preload_script(js_script).

I am planning to add these high level APIs for the script module:

1. pin()
2. unpin()
3. execute()
4. addDomMutationHandler()
5. removeDomMutationHandler()

These will also be available via the same driver instance - driver.script.pin(), is that good or should we do something else? I don't want to make the low-level API private since those can be used by users/other projects that uses selenium.

[shbenzer]

@shbenzer thanks for the review! I have one doubt, currently the low level APIs implemented in this PR are available via the driver instance like - driver.script.add_preload_script(js_script).

I am planning to add these high level APIs for the script module:

1. pin()
2. unpin()
3. execute()
4. addDomMutationHandler()
5. removeDomMutationHandler()

These will also be available via the same driver instance - driver.script.pin(), is that good or should we do something else? I don't want to make the low-level API private since those can be used by users/other projects that uses selenium.

methods prepended with _ are marked as private in python but are still accessible

[navin772]

methods prepended with _ are marked as private in python but are still accessible

yes, they are accessible, but I am not sure if that's a nice way to access the low level API, since we are not doing the _ private marking in other modules.

[shbenzer]

I used _ in the Network module, but I’m definitely open to other suggestions

[navin772]

@shbenzer I will add the _ prefix to make them low level API. Other option might be to add a prefix like bidi_add_preload_script() but since we already use _ in other module, it should be consistent.

[navin772]

The //java/test/org/openqa/selenium/bidi/storage:StorageCommandsTest-edge test is failing in the CI and recent runs on trunk which is unrelated to this PR. I tried debugging the test but it is passing locally. Probably a Edge browser version issue.