Skip to content

Conversation

pujagani
Copy link
Contributor

@pujagani pujagani commented Aug 14, 2025

User description

🔗 Related Issues

💥 What does this PR do?

Adds Emulation module support as per BiDi spec

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

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

PR Type

Enhancement


Description

  • Add BiDi Emulation module with geolocation override support

  • Implement GeolocationCoordinates class with validation and fluent API

  • Add comprehensive test coverage for browsing and user contexts

  • Update build configuration to include emulation module


Diagram Walkthrough

flowchart LR
  A["Emulation Module"] --> B["GeolocationCoordinates"]
  A --> C["setGeolocationOverride"]
  C --> D["Browsing Context"]
  C --> E["User Context"]
  F["Test Suite"] --> A
  G["Build Config"] --> A
Loading

File Walkthrough

Relevant files
Enhancement
emulation.js
Core Emulation module implementation                                         

javascript/selenium-webdriver/bidi/emulation/emulation.js

  • Create Emulation class with BiDi protocol integration
  • Implement setGeolocationOverride method for coordinates and error
    states
  • Add context and user context parameter validation
  • Include error handling for BiDi command responses
+94/-0   
geolocationCoordinates.js
GeolocationCoordinates class with validation                         

javascript/selenium-webdriver/bidi/emulation/geolocationCoordinates.js

  • Create GeolocationCoordinates class with fluent API design
  • Add validation for latitude, longitude, accuracy, altitude, heading,
    speed
  • Implement private Map storage with getter method
  • Include comprehensive JSDoc documentation with examples
+137/-0 
Tests
emulation_test.js
Comprehensive test coverage for Emulation                               

javascript/selenium-webdriver/test/bidi/emulation_test.js

  • Add test suite for Emulation module functionality
  • Test geolocation override for browsing contexts and user contexts
  • Include error state testing with GeolocationPositionError
  • Add permission setup and geolocation retrieval validation
+161/-0 
Configuration changes
BUILD.bazel
Build configuration update                                                             

javascript/selenium-webdriver/BUILD.bazel

  • Add bidi/emulation/*.js pattern to js_library sources
+1/-0     

@selenium-ci selenium-ci added C-nodejs JavaScript Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Aug 14, 2025
@pujagani pujagani marked this pull request as ready for review August 18, 2025 04:40
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

5678 - Partially compliant

Compliant requirements:

Non-compliant requirements:

  • Resolve/reduce ChromeDriver "ConnectFailure" instantiation errors.
  • Ensure stability across multiple ChromeDriver instances.
  • Add diagnostics/handling for driver connection lifecycle.

Requires further human verification:

  • Reproduce the ChromeDriver connection issue on the specified environment and versions.
  • Validate behavior across multiple driver instantiations outside of test harness.

1234 - Partially compliant

Compliant requirements:

Non-compliant requirements:

  • Ensure click() triggers JavaScript in href for affected Firefox versions.

Requires further human verification:

  • Reproduce behavior on target Firefox versions and confirm fix.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

API Consistency

The error constant GeolocationPositionError is a single fixed object with type: 'positionUnavailable'. Verify this shape and naming align with the BiDi spec and other language bindings (e.g., should different error types be supported, or different field names?).

const GeolocationPositionError = Object.freeze({
  type: 'positionUnavailable',
})

/**
Response Handling

The BiDi send response is only checked for response.type === 'error'. Confirm the actual response shape from bidi.send and consider try/catch or status validation to avoid unhandled protocol errors or unexpected shapes.

  const response = await this.bidi.send(command)

  if (response.type === 'error') {
    throw new Error(`${response.error}: ${response.message}`)
  }
}
Validation Bounds

Heading is validated as 0.0–360.0 inclusive; some specs treat 360 as equivalent to 0 or disallow 360. Confirm inclusive range and null handling match BiDi/Geolocation spec to prevent protocol rejections.

/**
 * Sets the heading in degrees.
 * @param {number|null} value - Heading (0.0 to 360.0) or null.
 * @returns {GeolocationCoordinates} This instance.
 * @throws {Error} If value is invalid.
 */
heading(value) {
  if (value !== null && (typeof value !== 'number' || value < 0.0 || value > 360.0)) {
    throw new Error(`Heading must be a number between 0.0 and 360.0. Received: '${value}'`)
  }
  this.#map.set('heading', value)
  return this
}

Copy link
Contributor

PR Code Suggestions ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related C-nodejs JavaScript Bindings Review effort 2/5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants