[java][BiDi] implement emulation.setScriptingEnabled - #16631

Follow the guide to enable codebase context checks.

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

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

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

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.

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

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.

@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

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.

@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

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:

  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);
  }
  // ...
}

+    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]

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

+  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]

-    // 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]