[java] Don't read the current window handle in newWindow (#17882)

[titusfortner]

🔗 Related Issues

Fixes #17882

💥 What does this PR do?

• Prevents calling switchTo().newWindow() without an active window from throwing an error
• Removes unnecessary behavior that switched back to original window if creating new window failed for some reason

🔧 Implementation Notes

• Brings Java behavior inline with all other bindings
• It's unclear what scenario this code was attempting to guard against, there isn't an obvious scenario it would be encountered

🤖 AI assistance

• AI assisted (complete below)
  • Tool(s): Claude Code
  • What was generated: reproduction, root-cause analysis, and the fix
  • I reviewed all AI output and can explain the change

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Summary by Qodo

Java: Avoid reading current window handle in switchTo().newWindow()

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Remove pre-fetch of current window handle when creating a new window/tab.
• Prevent newWindow() from failing when no active window exists.
• Update unit test expectations to match the new command sequence.

Diagram

graph TD
  A["RemoteWebDriver.newWindow()"] --> B["execute(SWITCH_TO_NEW_WINDOW)"] --> C[("Response.value.handle")]
  C --> D["switchTo().window(handle)"] --> E["RemoteWebDriver (returned)"]

  subgraph Legend
    direction LR
    _entry["API entrypoint"] ~~~ _cmd["Remote cmd"] ~~~ _data[("Returned handle")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Conditional fallback to original handle

• ➕ Preserves the previous safety-net behavior when an active window exists
• ➕ Could reduce user-visible state changes on failures in some environments
• ➖ Still requires calling GET_CURRENT_WINDOW_HANDLE, which can fail when no window is active (the reported bug)
• ➖ Adds branching complexity and diverges from other Selenium bindings’ behavior

2. Best-effort restore with ignored failures

• ➕ Avoids masking the original exception while attempting to revert focus
• ➕ Retains an attempt to return to the prior window when possible
• ➖ Introduces ambiguous behavior (sometimes restores, sometimes not)
• ➖ Still depends on reading or tracking the original handle, which is precisely the fragile step here

Recommendation: Keep the PR’s approach: remove reading/restoring the original window handle and rely on the remote end to create a new window and return its handle. This aligns Java with other bindings and eliminates a failure mode where GET_CURRENT_WINDOW_HANDLE throws when there is no active window, while keeping the command sequence minimal and predictable.

Files changed (2) +4 / -12

Bug fix (1) +4 / -11

RemoteWebDriver.javaSimplify newWindow() by removing original-handle read/restore +4/-11

Simplify newWindow() by removing original-handle read/restore

• Removes the preemptive getWindowHandle() call and the try/catch that attempted to switch back on failure. newWindow() now directly executes SWITCH_TO_NEW_WINDOW, switches to the returned handle, and returns the same driver instance.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java

Tests (1) +0 / -1

RemoteWebDriverUnitTest.javaUpdate unit test command expectations for newWindow() +0/-1

Update unit test command expectations for newWindow()

• Adjusts the expected command sequence to no longer include GET_CURRENT_WINDOW_HANDLE prior to SWITCH_TO_NEW_WINDOW, matching the simplified implementation.

java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. newWindowHandle line too long ✗ Dismissed 📘 Rule violation ⚙ Maintainability

Description

The new assignment to newWindowHandle is on a single long line that appears to violate the repo’s
Java formatting standards and may fail ./scripts/format.sh/CI formatting checks. This risks
introducing avoidable CI failures for an otherwise small functional change.

Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[1347]

+      String newWindowHandle = ((Map<String, Object>) response.getValue()).get("handle").toString();

Evidence

PR Compliance ID 8 requires changed files to conform to repository formatting checks. The added
newWindowHandle assignment is a single, very long chained expression that stands out from the
project’s typical formatted style and is likely to be rewritten by the formatter, causing CI
formatting failures.

AGENTS.md: Code must conform to repository formatting checks (scripts/format.sh)
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[1346-1349]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newly added Java line is likely not compliant with repository formatting checks and may fail `./scripts/format.sh`.

## Issue Context
The code uses a long chained expression on one line for `newWindowHandle`, whereas surrounding code generally follows formatter-driven wrapping.

## Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[1346-1349]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[titusfortner]

See reporting issue for reason