Skip to content

[java] remove try-catch while opening a new window #17754 - #17996

Merged
titusfortner merged 3 commits into
trunkfrom
new-window-fix
Sep 8, 2026
Merged

[java] remove try-catch while opening a new window #17754#17996
titusfortner merged 3 commits into
trunkfrom
new-window-fix

Conversation

@joerg1985

Copy link
Copy Markdown
Member

🔗 Related Issues

Fixes #17754

💥 What does this PR do?

Will allow to open a new window in case the current window context is invalid.

🔧 Implementation Notes

The try-catch block is not needed, either the switch to the new window fails and we should stay at the old window or the switch to the new window passes, no need to try to switch back to the old window.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

  • Bug fix (backwards compatible)

@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@selenium-ci selenium-ci added the C-java Java Bindings label Sep 7, 2026
@qodo-code-review

qodo-code-review Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. The Java window test now fails ✓ Resolved 📘 Rule violation ≡ Correctness
Description
newWindow no longer calls getWindowHandle(), but canHandleSwitchToNewWindowCommand still
requires GET_CURRENT_WINDOW_HANDLE before SWITCH_TO_NEW_WINDOW in its verified sequence.
Whenever the unit test runs, ordered interaction verification encounters the new-window command
while expecting the removed lookup, so it fails before covering window creation from an invalid or
closed current context.
Code

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

-      String original = getWindowHandle();
Evidence
The changed implementation begins with SWITCH_TO_NEW_WINDOW and then sends SWITCH_TO_WINDOW,
while the existing unit test still expects GET_CURRENT_WINDOW_HANDLE first. Because
verifyCommands performs strict, ordered Mockito verification and rejects missing or unexpected
interactions, this sequence mismatch makes the test fail; meanwhile, the available browser test
covers only a valid current context rather than the regression case where the active window has been
closed.

AGENTS.md: Use Focused, Reliable Tests and Avoid Contract-Misrepresenting Mocks
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[1345-1349]
java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java[314-326]
java/test/org/openqa/selenium/remote/WebDriverFixture.java[85-106]
java/test/org/openqa/selenium/WindowSwitchingTest.java[339-355]
java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java[315-325]

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

## Issue description
The Java unit test for opening a new window still expects the removed current-window-handle lookup, causing strict ordered command verification to fail.

## Issue Context
`RemoteWebDriver.newWindow` now sends `SWITCH_TO_NEW_WINDOW` followed by `SWITCH_TO_WINDOW` without first requesting `GET_CURRENT_WINDOW_HANDLE`. Update the existing unit-test expectation to match that sequence, and add regression coverage for opening a new window after the active window has been closed or the current context is otherwise invalid, if supported by the fixture; the available browser test currently covers only a valid current context.

## Fix Focus Areas
- java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java[314-326]
- java/test/org/openqa/selenium/WindowSwitchingTest.java[100-115]
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[1345-1349]

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


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This is a localized runtime change in WebDriver window-switching error handling, where exception and session-state behavior can affect a core API contract.

Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java
@titusfortner
titusfortner merged commit 96a38b5 into trunk Sep 8, 2026
30 checks passed
@titusfortner
titusfortner deleted the new-window-fix branch September 8, 2026 22:41
This was referenced Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: can not switch to new window after closing the current window

3 participants