[rb] BiDi get client windows

[aguspe]

User description

💥 What does this PR do?

This pull request adds support for listing and inspecting client windows through the Selenium BiDi (Bidirectional) protocol in Ruby.

It introduces a new class, WebDriver::BiDi::Browser::Window, which is a Struct that holds information about a browser window, including its handle, active state, and dimensions (height, width, x, y).

Adds a new method, windows, to the WebDriver::BiDi::Browser class. This method uses the browser.getClientWindows command to retrieve a list of all client windows and returns them as an array of Window objects.

I'm investigating how its suppose to work on Firefox, because it works as expected however Firefox doesn't seem to set the first window to active as Chrome does.

🔧 Implementation Notes

I've chosen to implement the Window class as a Struct because it's a lightweight, efficient way to bundle related data together without needing a full-blown class. This approach aligns well with how simple data objects are often handled in Ruby and provides a clean, predictable API for users.

I also added a helper method, active?, to the Window struct to make checking the active state more idiomatic for Ruby users.

🔄 Types of changes

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

---

PR Type

Enhancement

---

Description

• Add BiDi windows method to retrieve client windows

• Create Window struct with handle, active state, dimensions

• Add comprehensive test coverage for window operations

• Update Ruby type signatures for new functionality

---

Diagram Walkthrough

flowchart LR
  A["BiDi Browser"] --> B["windows() method"]
  B --> C["browser.getClientWindows command"]
  C --> D["Window struct creation"]
  D --> E["Window attributes mapping"]
  E --> F["active? helper method"]

Loading

File Walkthrough

	Relevant files
Enhancement	browser.rb Add BiDi client windows functionality                                        rb/lib/selenium/webdriver/bidi/browser.rb Add Window struct with handle, dimensions, and state properties Implement windows method to fetch client windows via BiDi Add active? helper method for window active state +23/-1
Formatting	browsing_context.rb Minor formatting improvement                                                          rb/lib/selenium/webdriver/bidi/browsing_context.rb Add comment for class closure consistency +1/-1
Tests	browser_spec.rb Add BiDi windows tests and refactor                                            rb/spec/integration/selenium/webdriver/bidi/browser_spec.rb Add comprehensive tests for windows method Test window attributes and active state checking Refactor test structure with shared setup Remove Firefox from browser support list +52/-33
Documentation	browser.rbs Update Ruby type signatures                                                            rb/sig/lib/selenium/webdriver/bidi/browser.rbs Add type signature for Window struct +2/-0

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 5678 - Partially compliant Compliant requirements: None Non-compliant requirements: Investigate and resolve ConnectFailure on multiple ChromeDriver instantiations Ensure subsequent ChromeDriver instantiations succeed Add reproducible steps or tests validating the fix Requires further human verification: Environment-specific validation on Ubuntu 16.04.4 with specified Chrome/ChromeDriver versions Runtime behavior across multiple instantiations outside CI 1234 - Partially compliant Compliant requirements: None Non-compliant requirements: Ensure click() triggers JavaScript in href for Firefox Provide tests verifying the behavior Requires further human verification: Browser-specific manual verification on Firefox 42 (legacy) if still relevant

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review API Shape The Window struct defines both an :active field and an active? method returning the raw value; consider clarity/consistency (boolean vs truthy) and whether additional helpers (e.g., state predicates) or documentation are needed. Window = Struct.new(:handle, :active, :height, :width, :x, :y, :state) do def active? active end end Debug Artifact The spec includes a pp call, which will clutter test output; remove or guard it. pp window_handle expect(browser.windows.first).to be_active Nil Safety windows relies on response['clientWindows']; consider guarding against nil or unexpected schema to avoid NoMethodError in edge cases. def windows response = @bidi.send_cmd('browser.getClientWindows') response['clientWindows'].map do |win_data| attributes = { handle: win_data['clientWindow'], active: win_data['active'], height: win_data['height'], width: win_data['width'], x: win_data['x'], y: win_data['y'], state: win_data['state'] } Window.new(**attributes) end end

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Fix Struct keyword initialization **Window is defined with positional fields but instantiated using keyword arguments (Window.new(attributes)), which will raise unless the Struct is created with keyword_init: true. Define the Struct as Struct.new(:handle, :active, ..., keyword_init: true) (or switch to positional initialization). Update the RBS to reflect keyword-based initialization and add the windows method signature returning Array[Window]. Examples: rb/lib/selenium/webdriver/bidi/browser.rb [24-59] Window = Struct.new(:handle, :active, :height, :width, :x, :y, :state) do def active? active end end def initialize(bidi) @bidi = bidi end def create_user_context ... (clipped 26 lines) rb/sig/lib/selenium/webdriver/bidi/browser.rbs [5-12] Window: Selenium::WebDriver::BiDi::Browser::Window @bidi: BiDi def initialize: (BiDi bidi) -> void def create_user_context: () -> Hash[String, String] Solution Walkthrough: Before: # rb/lib/selenium/webdriver/bidi/browser.rb class Browser # Struct expects positional arguments by default Window = Struct.new(:handle, :active, :height, :width, :x, :y, :state) def windows # ... response['clientWindows'].map do |win_data| attributes = { handle: win_data['clientWindow'], ... } # This will raise an ArgumentError Window.new(**attributes) end end end # rb/sig/lib/selenium/webdriver/bidi/browser.rbs # The signature for the new `windows` method is missing. After: # rb/lib/selenium/webdriver/bidi/browser.rb class Browser # Struct is now initialized with keyword arguments Window = Struct.new(:handle, :active, :height, :width, :x, :y, :state, keyword_init: true) def windows # ... response['clientWindows'].map do |win_data| attributes = { handle: win_data['clientWindow'], ... } # This now works correctly Window.new(**attributes) end end end # rb/sig/lib/selenium/webdriver/bidi/browser.rbs # Add the method signature for type checking def windows: () -> Array[Selenium::WebDriver::BiDi::Browser::Window] Suggestion importance[1-10]: 10 __ Why: This suggestion correctly identifies a critical bug where a positional Struct is initialized with keyword arguments, which would cause a runtime error, and also points out the missing method signature in the RBS file.	High
Possible issue	Enable keyword struct initialization **You're instantiating Window with keyword arguments in windows, but the struct isn't defined with keyword_init: true. This will raise an ArgumentError on Ruby 3+. Enable keyword initialization on the struct to match Window.new(attributes). rb/lib/selenium/webdriver/bidi/browser.rb [24-28] -Window = Struct.new(:handle, :active, :height, :width, :x, :y, :state) do +Window = Struct.new(:handle, :active, :height, :width, :x, :y, :state, keyword_init: true) do def active? active end end Apply / Chat Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies that Struct.new requires keyword_init: true to be initialized with keyword arguments as used in Window.new(**attributes), preventing a runtime ArgumentError on modern Ruby versions.	High
Learned best practice	Guard and coerce response fields Validate the response shape before mapping and coerce active to a boolean to avoid nils and NoMethodError on unexpected responses. rb/lib/selenium/webdriver/bidi/browser.rb [45-60] def windows response = @bidi.send_cmd('browser.getClientWindows') + return [] unless response.is_a?(Hash) && response['clientWindows'].is_a?(Array) response['clientWindows'].map do |win_data| attributes = { handle: win_data['clientWindow'], - active: win_data['active'], + active: !!win_data['active'], height: win_data['height'], width: win_data['width'], x: win_data['x'], y: win_data['y'], state: win_data['state'] } Window.new(**attributes) end end Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add defensive checks around parsing and collections to guard against missing keys and unexpected shapes before iterating.	Low
Update