Skip to content

[java][bidi] Handle user prompts during BiDi navigation #17244

Open
krishnamohan-kothapalli wants to merge 6 commits intoSeleniumHQ:trunkfrom
krishnamohan-kothapalli:java-bidi-navigation-clean
Open

[java][bidi] Handle user prompts during BiDi navigation #17244
krishnamohan-kothapalli wants to merge 6 commits intoSeleniumHQ:trunkfrom
krishnamohan-kothapalli:java-bidi-navigation-clean

Conversation

@krishnamohan-kothapalli

🔗 Related Issues

💥 What does this PR do?

When navigating via BiDi (get(), back(), forward(), refresh()), an
onload alert blocks the page from reaching its readiness state, causing
browsingContext.navigate/traverseHistory/reload to hang indefinitely.

This fix registers a browsingContext.userPromptOpened listener before each
BiDi navigation that automatically dismisses or accepts prompts according to
the unhandledPromptBehavior capability (defaulting to dismiss and notify).
The listener is removed in a finally block after navigation completes.

🔧 Implementation Notes

Classic WebDriver delegated prompt handling during navigation to the browser
via the unhandledPromptBehavior session capability. BiDi navigation commands
don't have that mechanism, so we replicate it by subscribing to
browsingContext.userPromptOpened for the duration of each navigation call.

The listener runs on the BiDi WebSocket receiver thread while sendAndWait
blocks the caller thread — the Connection implementation already handles
this pattern safely (see the tryLock comment in handleEventResponse).

IGNORE behaviour is preserved: navigation runs without a listener,
leaving the alert open for the caller to handle.

💡 Additional Considerations

  • Follow-up parity work may be needed in other bindings (Python, Ruby, .NET, JS)
    to replicate this behaviour for their BiDi navigation paths.

🔄 Types of changes

  • Bug fix (backwards compatible)

Krishna and others added 4 commits March 18, 2026 13:01
Implements get(), back(), forward(), and refresh() via
BrowsingContext when webSocketUrl capability is present,
falling back to Classic otherwise. Maps pageLoadStrategy
to ReadinessState: normal->COMPLETE, eager->INTERACTIVE,
none->NONE. Adds integration tests covering all navigation
commands with BiDi enabled.

Fixes SeleniumHQ#13995
- Guard BiDi path with instanceof HasBiDi check to prevent
  IllegalArgumentException on plain RemoteWebDriver instances
- Check webSocketUrl instanceof String (not just != null) to
  avoid false positive on Boolean.TRUE during session setup
- Handle PageLoadStrategy as both enum and String when mapping
  to ReadinessState to fix eager/none being silently ignored

Addresses review feedback on SeleniumHQ#13995
@selenium-ci selenium-ci added C-java Java Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Mar 21, 2026
@qodo-code-review
Copy link
Contributor

Review Summary by Qodo

Handle user prompts during BiDi navigation with automatic dismissal

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Handle user prompts during BiDi navigation via listener
• Route navigation commands (get, back, forward, refresh) through BiDi
• Map pageLoadStrategy to ReadinessState for BiDi compatibility
• Automatically dismiss or accept alerts per unhandledPromptBehavior capability

Grey Divider

File Changes

1. java/src/org/openqa/selenium/remote/RemoteWebDriver.java ✨ Enhancement +125/-4

Implement BiDi navigation with automatic prompt handling

• Added BiDi navigation support for get(), back(), forward(), and refresh() methods
• Implemented navigateViaBiDi() wrapper that registers a browsingContext.userPromptOpened
 listener to handle alerts according to unhandledPromptBehavior capability
• Added helper methods: isBiDiEnabled(), getReadinessState(), and
 getUnhandledPromptBehaviour() to manage BiDi navigation state
• Imported necessary BiDi classes and JSON utilities for event handling

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


2. java/test/org/openqa/selenium/bidi/browsingcontext/BiDiNavigationTest.java 🧪 Tests +113/-0

Add BiDi navigation integration tests

• Created comprehensive test suite for BiDi navigation functionality
• Tests cover driver.get(), navigate().to() with both String and URL objects
• Tests verify browser history traversal via back() and forward() methods
• Tests validate page reload via refresh() method

java/test/org/openqa/selenium/bidi/browsingcontext/BiDiNavigationTest.java


3. java/src/org/openqa/selenium/remote/BUILD.bazel ⚙️ Configuration changes +1/-0

Add BiDi browsingcontext dependency

• Added dependency on //java/src/org/openqa/selenium/bidi/browsingcontext module

java/src/org/openqa/selenium/remote/BUILD.bazel


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 21, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. BiDi ignores pageLoadTimeout🐞 Bug ✓ Correctness
Description
RemoteWebDriver.get/refresh now wait for BiDi navigation using the WebSocket timeout (default 30s)
rather than the configured pageLoadTimeout, which can cause unexpected timeouts or hangs relative to
classic navigation. This occurs because BrowsingContext.navigate/reload are invoked without a
Duration, so BiDi.send uses ClientConfig.wsTimeout instead of WebDriver timeouts.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R382-387]

+    if (isBiDiEnabled()) {
+      String contextId = getWindowHandle();
+      navigateViaBiDi(
+          contextId,
+          () -> new BrowsingContext(this, contextId).navigate(url, getReadinessState()));
+    } else {
Evidence
RemoteWebDriver.get calls BrowsingContext.navigate(url, readiness) without a Duration.
BrowsingContext.navigate delegates to BiDi.send(command), which blocks using the BiDi instance
timeout; that timeout is created from ClientConfig.wsTimeout (default 30 seconds). RemoteWebDriver
still supports setting pageLoadTimeout via DriverCommand.SET_PAGE_LOAD_TIMEOUT, but the BiDi path
does not consult it, so the configured page load timeout no longer governs navigation when BiDi is
enabled.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[380-389]
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[1242-1254]
java/src/org/openqa/selenium/bidi/browsingcontext/BrowsingContext.java[128-138]
java/src/org/openqa/selenium/bidi/BiDi.java[60-69]
java/src/org/openqa/selenium/bidi/BiDiProvider.java[74-85]
java/src/org/openqa/selenium/remote/http/ClientConfig.java[100-103]

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

## Issue description
When BiDi is enabled, `RemoteWebDriver.get()`/`refresh()` call `BrowsingContext.navigate/reload` without passing a `Duration`. This means the wait is governed by the BiDi WebSocket timeout (default 30s) instead of the session's configured `pageLoadTimeout`.
### Issue Context
- `RemoteTimeouts.pageLoadTimeout(Duration)` still sends `SET_PAGE_LOAD_TIMEOUT`, but the BiDi navigation path never reads or uses that value.
- `BrowsingContext` already exposes overloads that accept a `Duration` for `navigate`.
### Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[380-475]
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[1242-1255]
- java/src/org/openqa/selenium/bidi/browsingcontext/BrowsingContext.java[128-145]
### Implementation notes
- Introduce a helper in `RemoteWebDriver` to obtain the effective page load timeout (either cache the last-set value in the driver, initialize from session timeouts, or reuse the existing `GET_TIMEOUTS` parsing logic).
- Pass that `Duration` into `BrowsingContext.navigate(url, readiness, timeout)` and `BrowsingContext.reload(readiness, timeout)` (if needed, add a reload overload that accepts `Duration`).
- Add/extend tests to validate that BiDi navigation honors a non-default pageLoadTimeout.

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



Remediation recommended

2. *_AND_NOTIFY never notifies 📘 Rule violation ✓ Correctness
Description
The new BiDi navigation prompt handling treats ACCEPT_AND_NOTIFY/DISMISS_AND_NOTIFY the same as
ACCEPT/DISMISS, so callers are never notified (e.g., via UnhandledAlertException) that a
prompt was present. This changes user-visible unhandledPromptBehavior semantics and can break code
relying on the ...AND_NOTIFY behavior.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R445-454]

+    UnexpectedAlertBehaviour behaviour = getUnhandledPromptBehaviour();
+    if (behaviour == UnexpectedAlertBehaviour.IGNORE) {
+      navigation.run();
+      return;
+    }
+
+    boolean accept =
+        behaviour == UnexpectedAlertBehaviour.ACCEPT
+            || behaviour == UnexpectedAlertBehaviour.ACCEPT_AND_NOTIFY;
+
Evidence
PR Compliance ID 1 requires preserving user-facing behavior; however, the implementation collapses
ACCEPT_AND_NOTIFY into accept=true and does not implement any distinct 'notify' behavior for
either *_AND_NOTIFY variant (no exception/error propagation after handling the prompt).

AGENTS.md
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[445-474]

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

## Issue description
`navigateViaBiDi` currently treats `ACCEPT_AND_NOTIFY`/`DISMISS_AND_NOTIFY` the same as `ACCEPT`/`DISMISS`, so users are not notified when a prompt appeared during BiDi navigation.
## Issue Context
`unhandledPromptBehavior` capability includes explicit `...AND_NOTIFY` variants; callers may rely on an error/exception signal (e.g., `UnhandledAlertException`) to detect that an unexpected prompt occurred.
## Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[428-475]

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


3. No test for prompt navigation📘 Rule violation ⛯ Reliability
Description
The PR adds new behavior to auto-handle browsingContext.userPromptOpened during BiDi navigation,
but the added/updated tests only validate navigation success and do not cover the prompt-handling
regression scenario. This risks reintroducing hangs or incorrect prompt handling without detection.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R440-475]

+  // Wraps a BiDi navigation call with a userPromptOpened listener that handles any alerts
+  // according to the unhandledPromptBehavior capability. This replicates, for BiDi navigation,
+  // the automatic prompt handling that classic WebDriver delegates to the browser via the
+  // capability.
+  private void navigateViaBiDi(String contextId, Runnable navigation) {
+    UnexpectedAlertBehaviour behaviour = getUnhandledPromptBehaviour();
+    if (behaviour == UnexpectedAlertBehaviour.IGNORE) {
+      navigation.run();
+      return;
+    }
+
+    boolean accept =
+        behaviour == UnexpectedAlertBehaviour.ACCEPT
+            || behaviour == UnexpectedAlertBehaviour.ACCEPT_AND_NOTIFY;
+
+    BiDi bidi = ((HasBiDi) this).getBiDi();
+    long listenerId =
+        bidi.addListener(
+            contextId,
+            USER_PROMPT_OPENED_EVENT,
+            prompt -> {
+              if (contextId.equals(prompt.getBrowsingContextId())) {
+                LOG.fine(
+                    () ->
+                        String.format(
+                            "Handling %s user prompt during BiDi navigation (%s)",
+                            prompt.getType(), accept ? "accept" : "dismiss"));
+                new BrowsingContext(this, contextId).handleUserPrompt(accept);
+              }
+            });
+    try {
+      navigation.run();
+    } finally {
+      bidi.removeListener(listenerId);
+    }
}
Evidence
PR Compliance ID 4 requires changed behavior to be covered by tests; the production change
specifically adds a userPromptOpened listener during navigation, but the new BiDiNavigationTest
exercises only basic navigation paths and includes no page/flow that triggers an onload
alert/prompt.

AGENTS.md
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[440-475]
java/test/org/openqa/selenium/bidi/browsingcontext/BiDiNavigationTest.java[35-113]

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

## Issue description
Add a regression test that triggers an onload alert/prompt during BiDi navigation and asserts navigation completes (and that the prompt is handled according to `unhandledPromptBehavior`).
## Issue Context
The reported bug is that an `onload` alert prevents BiDi navigation readiness from completing and can hang indefinitely; this should be covered by a dedicated test page like `common/src/web/pageWithOnLoad.html` or `common/src/web/slowLoadingAlert.html`.
## Fix Focus Areas
- java/test/org/openqa/selenium/bidi/browsingcontext/BiDiNavigationTest.java[1-113]
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[381-475]

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


4. Subscription leak after navigation🐞 Bug ➹ Performance
Description
navigateViaBiDi calls BiDi.addListener which sends a browser-side session.subscribe on every
navigation, but cleanup only calls BiDi.removeListener(listenerId) which never sends
session.unsubscribe. This leaves the remote end subscribed to browsingContext.userPromptOpened for
the rest of the session and causes redundant event traffic over time.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R455-474]

+    BiDi bidi = ((HasBiDi) this).getBiDi();
+    long listenerId =
+        bidi.addListener(
+            contextId,
+            USER_PROMPT_OPENED_EVENT,
+            prompt -> {
+              if (contextId.equals(prompt.getBrowsingContextId())) {
+                LOG.fine(
+                    () ->
+                        String.format(
+                            "Handling %s user prompt during BiDi navigation (%s)",
+                            prompt.getType(), accept ? "accept" : "dismiss"));
+                new BrowsingContext(this, contextId).handleUserPrompt(accept);
+              }
+            });
+    try {
+      navigation.run();
+    } finally {
+      bidi.removeListener(listenerId);
+    }
Evidence
BiDi.addListener performs a session.subscribe command before registering the local callback.
BiDi.removeListener only removes the callback from Connection; neither BiDi.removeListener nor
Connection.removeListener issues session.unsubscribe. Unsubscribe is only performed in
Connection.clearListeners during BiDi.close, so the temporary subscription established for
navigation persists until session end.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[455-474]
java/src/org/openqa/selenium/bidi/BiDi.java[80-91]
java/src/org/openqa/selenium/bidi/BiDi.java[117-119]
java/src/org/openqa/selenium/bidi/Connection.java[203-220]
java/src/org/openqa/selenium/bidi/Connection.java[232-251]

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

## Issue description
`navigateViaBiDi` intends a temporary `userPromptOpened` listener, but it only removes the local callback. Because `addListener` sends `session.subscribe` and `removeListener` never sends `session.unsubscribe`, the browser remains subscribed for the lifetime of the session.
### Issue Context
- `BiDi.addListener(..., browsingContextId, ...)` always sends `session.subscribe`.
- `BiDi.removeListener(id)` delegates to `Connection.removeListener` (local bookkeeping only).
### Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[455-474]
- java/src/org/openqa/selenium/bidi/BiDi.java[71-119]
- java/src/org/openqa/selenium/bidi/Connection.java[203-252]
### Implementation notes
Choose one:
- Improve BiDi/Connection listener lifecycle: when the last listener for an Event is removed, send `session.unsubscribe` for that event.
- Or change `navigateViaBiDi` to maintain a single long-lived subscription and only enable/disable handling during navigation (so you don't repeatedly subscribe).
- Add tests (or logging assertions) to ensure subscriptions do not accumulate across multiple navigations.

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


View more (1)
5. Back/forward ignore readiness state🐞 Bug ✓ Correctness
Description
RemoteWebDriver.Navigation.back/forward use BrowsingContext.back/forward, but these BiDi methods do
not accept or apply the ReadinessState derived from pageLoadStrategy, so BiDi history traversal
cannot honor NONE/EAGER semantics. This makes back/forward behavior inconsistent with
get()/refresh(), which do use getReadinessState().
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R1318-1333]

+      if (isBiDiEnabled()) {
+        String contextId = getWindowHandle();
+        navigateViaBiDi(
+            contextId, () -> new BrowsingContext(RemoteWebDriver.this, contextId).back());
+      } else {
+        execute(DriverCommand.GO_BACK);
+      }
  }
  @Override
  public void forward() {
-      execute(DriverCommand.GO_FORWARD);
+      if (isBiDiEnabled()) {
+        String contextId = getWindowHandle();
+        navigateViaBiDi(
+            contextId, () -> new BrowsingContext(RemoteWebDriver.this, contextId).forward());
+      } else {
Evidence
RemoteWebDriver derives a ReadinessState from pageLoadStrategy for get/refresh, but back/forward
call BrowsingContext.back/forward with no readiness/wait parameter. BrowsingContext.back/forward
delegate to traverseHistory(delta) which only sends delta/context and has no way to incorporate the
chosen readiness state.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[399-413]
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[1314-1336]
java/src/org/openqa/selenium/bidi/browsingcontext/BrowsingContext.java[387-398]

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

## Issue description
BiDi history traversal (`back()`/`forward()`) does not apply the same ReadinessState mapping used by `get()`/`refresh()`, so `pageLoadStrategy` can't influence back/forward behavior.
### Issue Context
- `RemoteWebDriver.getReadinessState()` maps pageLoadStrategy to `ReadinessState`.
- `BrowsingContext.back/forward` currently just call `traverseHistory(delta)` with no wait/readiness.
### Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[1314-1360]
- java/src/org/openqa/selenium/bidi/browsingcontext/BrowsingContext.java[387-398]
### Implementation notes
- Extend `BrowsingContext.traverseHistory`/`back`/`forward` to accept a `ReadinessState` (and ideally a `Duration` timeout), if supported by the BiDi command.
- Update `RemoteWebDriver.RemoteNavigation.back/forward` to pass `getReadinessState()` similarly to get/refresh.
- Add tests for pageLoadStrategy NONE/EAGER affecting back/forward behavior.

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



Advisory comments

6. USER_PROMPT_OPENED_EVENT comment restates 📘 Rule violation ⚙ Maintainability
Description
The comment above USER_PROMPT_OPENED_EVENT describes what the code is rather than why it exists or
what tradeoff it addresses. This reduces maintainability by not capturing intent (e.g., why it must
be shared/static or why the custom parsing is needed).
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R417-418]

+  // Shared event definition for browsingContext.userPromptOpened used during navigation.
+  private static final Event<UserPromptOpened> USER_PROMPT_OPENED_EVENT =
Evidence
PR Compliance ID 7 requires comments to explain rationale; the added comment is a 'what' description
of the field rather than 'why' it is structured this way.

AGENTS.md
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[417-418]

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 comment for `USER_PROMPT_OPENED_EVENT` restates the code rather than explaining intent.
## Issue Context
This code introduces a shared event definition and custom parsing; future maintainers need to know why it is shared/static and why parsing is implemented this way.
## Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[415-427]

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

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-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants