[java][BiDi] implement emulation.setScriptingEnabled

[Delta456]

User description

🔗 Related Issues

💥 What does this PR do?

Implements https://w3c.github.io/webdriver-bidi/#command-emulation-setScriptingEnabled for Java

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

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

---

PR Type

Enhancement

---

Description

• Implements BiDi emulation.setScriptingEnabled command for Java

• Adds SetScriptingEnabledParameters class with validation logic

• Includes comprehensive tests for contexts and user contexts

• Adds test HTML page for script execution verification

---

Diagram Walkthrough

flowchart LR
  A["Emulation.java"] -- "adds method" --> B["setScriptingEnabled"]
  C["SetScriptingEnabledParameters.java"] -- "new class" --> B
  D["SetScriptingEnabledTest.java"] -- "tests" --> B
  E["script_page.html"] -- "test resource" --> D

Loading

File Walkthrough

	Relevant files
Enhancement	Emulation.java Add setScriptingEnabled method to Emulation                            java/src/org/openqa/selenium/bidi/emulation/Emulation.java Adds setScriptingEnabled() method to Emulation class Sends BiDi command emulation.setScriptingEnabled with parameters Validates non-null parameters before sending +6/-0      SetScriptingEnabledParameters.java New SetScriptingEnabledParameters class with validation    java/src/org/openqa/selenium/bidi/emulation/SetScriptingEnabledParameters.java New parameter class extending AbstractOverrideParameters Validates that enabled parameter is false or null only Supports contexts() and userContexts() fluent methods Throws IllegalArgumentException if enabled is true +44/-0
Tests	SetScriptingEnabledTest.java Comprehensive tests for setScriptingEnabled functionality java/test/org/openqa/selenium/bidi/emulation/SetScriptingEnabledTest.java Tests setScriptingEnabled with contexts parameter Tests setScriptingEnabled with user contexts parameter Verifies JavaScript execution is disabled when enabled=false Verifies JavaScript execution is re-enabled when enabled=null +126/-0  script_page.html Test page for script execution verification                            common/src/web/script_page.html New test HTML page with inline script setting window.foo variable Used to verify script execution state in tests +9/-0

---

[qodo-merge-pro]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡	🎫 #5678 🔴 Investigate and resolve repeated "Error: ConnectFailure (Connection refused)" when instantiating multiple ChromeDriver instances on Ubuntu with specified versions. Provide guidance or code changes that prevent the connection refusal after the first ChromeDriver instance. Ensure reliability across multiple driver instantiations without console errors.
🟡	🎫 #1234 🔴 Ensure WebDriver click() triggers JavaScript in link href for Firefox as in 2.47.1 behavior. Provide fix or regression handling for Firefox versions where JS in href is not executed on click(). Include tests verifying click() triggers href JavaScript.
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logging: The new method setScriptingEnabled performs a critical configuration change without emitting any audit log including who initiated it, when, and the outcome. Referred Code public Map<String, Object> setScriptingEnabled(SetScriptingEnabledParameters parameters) { Require.nonNull("SetScriptingEnabled parameters", parameters); return bidi.send(new Command<>("emulation.setScriptingEnabled", parameters.toMap(), Map.class)); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Limited validation: Constructor throws a generic IllegalArgumentException for enabled=true but does not validate or guard against null/empty lists in contexts() and userContexts() which are external inputs. Referred Code @Override public SetScriptingEnabledParameters contexts(java.util.List<String> contexts) { super.contexts(contexts); return this; } @Override public SetScriptingEnabledParameters userContexts(java.util.List<String> userContexts) { super.userContexts(userContexts); return this; } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Input validation gaps: Methods contexts() and userContexts() accept external identifiers without validating format, emptiness, or duplicates, which may lead to unexpected behavior downstream. Referred Code @Override public SetScriptingEnabledParameters contexts(java.util.List<String> contexts) { super.contexts(contexts); return this; } @Override public SetScriptingEnabledParameters userContexts(java.util.List<String> userContexts) { super.userContexts(userContexts); return this; } Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Partially implement spec without justification The implementation of setScriptingEnabled partially follows the W3C spec by disallowing enabled: true. It should either be fully implemented to support true or the limitation should be documented. Examples: java/src/org/openqa/selenium/bidi/emulation/SetScriptingEnabledParameters.java [21-31] public SetScriptingEnabledParameters(Boolean enabled) { // allow null if (enabled == null) { map.put("enabled", null); } else if (!enabled) { map.put("enabled", enabled); } else { throw new IllegalArgumentException( "Only emulation of disabled JavaScript is supported (enabled must be false or null)"); } ... (clipped 1 lines) Solution Walkthrough: Before: // In SetScriptingEnabledParameters.java public class SetScriptingEnabledParameters extends AbstractOverrideParameters { public SetScriptingEnabledParameters(Boolean enabled) { if (enabled == null) { map.put("enabled", null); } else if (!enabled) { // only allows false map.put("enabled", enabled); } else { // enabled == true throw new IllegalArgumentException( "Only emulation of disabled JavaScript is supported (enabled must be false or null)"); } } // ... } After: // In SetScriptingEnabledParameters.java public class SetScriptingEnabledParameters extends AbstractOverrideParameters { /** * @param enabled `false` to disable scripting, `true` to enable it, * or `null` to clear the override and revert to the default state. */ public SetScriptingEnabledParameters(Boolean enabled) { // This fully aligns with the spec, allowing true, false, or null. map.put("enabled", enabled); } // ... } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the implementation deviates from the W3C specification by disallowing enabled: true, which is a valid and important point regarding API design and spec compliance.	Medium
Learned best practice	✅ Cleanup created user contexts Suggestion Impact: The commit added cleanup of the created user context at the end of the test by calling browser.removeUserContext(userContext), addressing the resource leak concern, though not using a finally block. code diff: + browser.removeUserContext(userContext); } Ensure created user contexts are cleaned up after the test by deleting them in a finally block to prevent resource leaks. java/test/org/openqa/selenium/bidi/emulation/SetScriptingEnabledTest.java [82-113] String userContext = browser.createUserContext(); -BrowsingContext context = - new BrowsingContext( - driver, new CreateContextParameters(WindowType.TAB).userContext(userContext)); -String contextId = context.getId(); -... -// Clear the scripting override -emulation.setScriptingEnabled( - new SetScriptingEnabledParameters(null).userContexts(List.of(userContext))); +try { + BrowsingContext context = + new BrowsingContext( + driver, new CreateContextParameters(WindowType.TAB).userContext(userContext)); + String contextId = context.getId(); + ... + emulation.setScriptingEnabled( + new SetScriptingEnabledParameters(null).userContexts(List.of(userContext))); +} finally { + browser.deleteUserContext(userContext); +} [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Ensure test setup/teardown and resource cleanup to avoid leaking browser contexts or user contexts.	Low
	Improve exception clarity Clarify the exception message to explicitly state the unsupported value and accepted values to aid debugging and API consumers. java/src/org/openqa/selenium/bidi/emulation/SetScriptingEnabledParameters.java [27-30] } else { throw new IllegalArgumentException( - "Only emulation of disabled JavaScript is supported (enabled must be false or null)"); + "Unsupported value for 'enabled': true. Accepted values are false (to disable) or null (to clear override)."); } Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Guard external API parameters with clear validation messages that reflect supported behavior.	Low
General	✅ Extract duplicated code into a helper Suggestion Impact: A private helper method isFooInWindow was added and the duplicated inline evaluation code was replaced with calls to this helper. code diff: + private boolean isFooInWindow(String contextId, Script script) { + EvaluateResult result = + script.evaluateFunctionInBrowsingContext( + contextId, "'foo' in window", false, Optional.empty()); + return (Boolean) ((EvaluateResultSuccess) result).getResult().getValue().get(); + } + Extract the duplicated script evaluation logic for checking if 'foo' in window into a private helper method to reduce code repetition and improve test readability. java/test/org/openqa/selenium/bidi/emulation/SetScriptingEnabledTest.java [57-74] -EvaluateResult resultDisabled = - script.evaluateFunctionInBrowsingContext( - contextId, "'foo' in window", false, Optional.empty()); -Boolean valueDisabled = - (Boolean) ((EvaluateResultSuccess) resultDisabled).getResult().getValue().get(); -assertThat(valueDisabled).isFalse(); +assertThat(isFooInWindow(contextId, script)).isFalse(); emulation.setScriptingEnabled( new SetScriptingEnabledParameters(null).contexts(List.of(contextId))); context.navigate(appServer.whereIs("script_page.html"), ReadinessState.COMPLETE); -EvaluateResult resultEnabled = - script.evaluateFunctionInBrowsingContext( - contextId, "'foo' in window", false, Optional.empty()); -Boolean valueEnabled = - (Boolean) ((EvaluateResultSuccess) resultEnabled).getResult().getValue().get(); -assertThat(valueEnabled).isTrue(); +assertThat(isFooInWindow(contextId, script)).isTrue(); [Suggestion processed] Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies duplicated code in the test and proposes extracting it into a helper method, which improves code readability and maintainability.	Low
	✅ Simplify constructor logic for clarity Suggestion Impact: The commit replaced the multi-branch conditional with a single true-check (enabled == Boolean.TRUE) followed by map.put("enabled", enabled), matching the suggested simplification. code diff: - // allow null - if (enabled == null) { - map.put("enabled", null); - } else if (!enabled) { - map.put("enabled", enabled); - } else { + if (enabled == Boolean.TRUE) { throw new IllegalArgumentException( "Only emulation of disabled JavaScript is supported (enabled must be false or null)"); } + map.put("enabled", enabled); // null or false Simplify the constructor logic by first checking for the invalid true case for the enabled parameter, then using a single map.put call for both null and false values. java/src/org/openqa/selenium/bidi/emulation/SetScriptingEnabledParameters.java [22-30] -// allow null -if (enabled == null) { - map.put("enabled", null); -} else if (!enabled) { - map.put("enabled", enabled); -} else { +if (enabled != null && enabled) { throw new IllegalArgumentException( "Only emulation of disabled JavaScript is supported (enabled must be false or null)"); } +map.put("enabled", enabled); [Suggestion processed] Suggestion importance[1-10]: 4 __ Why: The suggestion correctly simplifies the constructor's conditional logic, making the code more concise and readable without changing its behavior.	Low
Update

[navin772]

Check comments by the AI bot, these makes sense:

1. Cleanup created user contexts
2. Extract duplicated code into a helper
3. Simplify constructor logic for clarity

But for 3rd one, we should have this:

public class SetScriptingEnabledParameters extends AbstractOverrideParameters {
  public SetScriptingEnabledParameters(Boolean enabled) {
    if (enabled == Boolean.TRUE) {
      throw new IllegalArgumentException(
          "Only emulation of disabled JavaScript is supported (enabled must be false or null)");
    }

    map.put("enabled", enabled); // null or false
  }
}

[asolntsev]

@Delta456 After setting "js enabled = false", how can I reset it back?

[Delta456]

@Delta456 After setting "js enabled = false", how can I reset it back?

You need to use enabled = None to reset it back. Check the tests.

[asolntsev]

I don't see the word "None" in the PR. :)

I see only these two variants:

1. emulation.setScriptingEnabled( new SetScriptingEnabledParameters(false))
2. emulation.setScriptingEnabled( new SetScriptingEnabledParameters(null))

So when I want "true", I have to write "null" instead of "true". Is it logical?

Why can't we allow users write "true"?

[Delta456]

I don't see the word "None" in the PR. :)

I see only these two variants:

1. `emulation.setScriptingEnabled( new SetScriptingEnabledParameters(false))`

2. `emulation.setScriptingEnabled( new SetScriptingEnabledParameters(null))`

So when I want "true", I have to write "null" instead of "true". Is it logical?

Why can't we allow users write "true"?

Ah, my bad. I meant null.

We can allow users to use true but we also need to follow the W3C spec.

As per the spec, this command is used to disable the JavaScript on webpages

[asolntsev]

Ah yes, now I see it. W3C spec allows only false or null. This is a very strange spec IMHO. :)

[asolntsev]

Good!

[diemol]

Thank you, @Delta456!

[navin772]

LGTM!