Skip to content

[java][BiDi] change emulation commands return type to void #16699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
asolntsev merged 1 commit into from
Dec 7, 2025
Merged

[java][BiDi] change emulation commands return type to void #16699

asolntsev merged 1 commit into from
Dec 7, 2025

Conversation

@Delta456
Copy link
Member

@Delta456 Delta456 commented Dec 7, 2025 •
edited by qodo-code-review bot
Loading

User description

🔗 Related Issues

💥 What does this PR do?

All commands return Empty not Map<String, Object> which was a mistake on my part.

Spec: https://w3c.github.io/webdriver-bidi/#module-emulation

Thanks to @asolntsev for noticing this in my previous PR

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Bug fix

Description

  • Corrects emulation command return types from Map to void

  • Aligns implementation with W3C WebDriver BiDi specification

  • Fixes inconsistency where three methods incorrectly returned Map<String, Object>

Diagram Walkthrough

flowchart LR
  A["Emulation Methods<br/>Return Map&lt;String, Object&gt;"] -->|"Change to void"| B["Emulation Methods<br/>Return void"]
  C["W3C BiDi Spec<br/>Requires void"] -.->|"Compliance"| B
Loading

File Walkthrough
Relevant files
Bug fix
Emulation.java
Correct emulation method return types to void                       

java/src/org/openqa/selenium/bidi/emulation/Emulation.java

  • Changed setGeolocationOverride() return type from Map
    to void
  • Changed setTimezoneOverride() return type from Map to
    void
  • Changed setScriptingEnabled() return type from Map to
    void
  • Removed return statements from method bodies to match void signature
 +6/-7     

@Delta456 Delta456 requested a review from asolntsev December 7, 2025 18:35
@selenium-ci selenium-ci added C-java Java Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Dec 7, 2025
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Dec 7, 2025

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 ChromeDriver "ConnectFailure (Connection refused)" errors
occurring on subsequent driver instantiations in Ubuntu 16.04 with Selenium 3.9.0 and
Chrome 65/ChromeDriver 2.35.
Provide a fix or guidance so that only the first ChromeDriver instance is not uniquely
successful; subsequent instantiations should also connect without errors.
Validate behavior with the specified environment and reproduce the issue using the
provided steps/screenshots.
🟡
🎫 #1234
🔴 Ensure click() triggers JavaScript in href for Firefox (noted regression from 2.47.1 to
2.48.x).
Reproduce and validate behavior using the provided test case/videos on Firefox 42.
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: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

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 auditing: The new void emulation methods invoke BiDi commands without any added auditing or logging
of the action, user, or outcome, making it unclear whether critical actions are captured
elsewhere.

Referred Code 
public void setGeolocationOverride(SetGeolocationOverrideParameters parameters) {
  Require.nonNull("SetGeolocationOverride parameters", parameters);

  bidi.send(new Command<>("emulation.setGeolocationOverride", parameters.toMap(), Map.class));
}

public void setTimezoneOverride(SetTimezoneOverrideParameters parameters) {
  Require.nonNull("SetTimezoneOverride parameters", parameters);

  bidi.send(new Command<>("emulation.setTimezoneOverride", parameters.toMap(), Map.class));
}

public void setScriptingEnabled(SetScriptingEnabledParameters parameters) {
  Require.nonNull("SetScriptingEnabled parameters", parameters);

  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:
Unhandled errors: The methods send BiDi commands without visible handling of failures, exceptions, or edge
cases (e.g., transport errors), which may be addressed by lower layers but is not evident
from the diff.

Referred Code 
public void setGeolocationOverride(SetGeolocationOverrideParameters parameters) {
  Require.nonNull("SetGeolocationOverride parameters", parameters);

  bidi.send(new Command<>("emulation.setGeolocationOverride", parameters.toMap(), Map.class));
}

public void setTimezoneOverride(SetTimezoneOverrideParameters parameters) {
  Require.nonNull("SetTimezoneOverride parameters", parameters);

  bidi.send(new Command<>("emulation.setTimezoneOverride", parameters.toMap(), Map.class));
}

public void setScriptingEnabled(SetScriptingEnabledParameters parameters) {
  Require.nonNull("SetScriptingEnabled parameters", parameters);

  bidi.send(new Command<>("emulation.setScriptingEnabled", parameters.toMap(), Map.class));
}

Learn more about managing compliance generic rules or creating your own custom rules
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Dec 7, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use Void.class for empty responses

Replace Map.class with Void.class in bidi.send calls for methods with a void
return type to avoid unnecessary deserialization of an empty response.

java/src/org/openqa/selenium/bidi/emulation/Emulation.java [43]

-bidi.send(new Command<>("emulation.setGeolocationOverride", parameters.toMap(), Map.class));
+bidi.send(new Command<>("emulation.setGeolocationOverride", parameters.toMap(), Void.class));
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that using Void.class is more precise and potentially more efficient than Map.class for a command where the result is ignored, improving code clarity and performance.

Low
  • More

@asolntsev asolntsev merged commit cfbeed8 into SeleniumHQ:trunk Dec 7, 2025
41 of 42 checks passed
@Delta456 Delta456 deleted the emul_void branch December 8, 2025 02:14
This was referenced Jan 19, 2026
Open
Open
Open
Merged
Merged
Open
Merged
Open
Open
Open
Open
Closed
Merged
Merged
Merged
Closed
Closed
Merged
Closed
Merged
This was referenced Jan 19, 2026
Closed
Merged
Open
Merged
Closed
Open
Merged
This was referenced Feb 1, 2026
Closed
Closed
@dependabot dependabot bot mentioned this pull request Feb 11, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asolntsev asolntsev asolntsev approved these changes

Assignees

No one assigned

Labels

B-devtools Includes everything BiDi or Chrome DevTools related C-java Java Bindings Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Delta456 @asolntsev @selenium-ci