[py] fix add_request_handler ignoring url_patterns on callback= path

[titusfortner]

🔗 Related Issues

💥 What does this PR do?

Fixes the high-level driver.network.add_request_handler(callback=...) form so it honors url_patterns. Previously, passing the handler as a keyword left event=None, which dropped the patterns and intercepted every request instead of only the ones the caller scoped.

🔧 Implementation Notes

The keyword (callback=) and positional dispatch paths are now consistent — when no positional event is given, the intercept falls back to url_patterns.

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s): Claude Code (Opus 4.8)
  • What was generated: the fix and its regression test
  • I reviewed all AI output and can explain the change

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

1. Dict patterns match everything 🐞 Bug ≡ Correctness

Description

When add_request_handler(callback=..., url_patterns=...) is used and url_patterns contains
wire-level dict UrlPatterns, the high-level registry does not apply any Python-side URL filtering
(it only compiles regexes for string globs), so the callback can run for unrelated requests that
weren't blocked by the intercept.

Code

py/private/bidi_enhancements_manifest.py[R1179-1180]

+                patterns = event if event is not None else url_patterns
+                return self._request_handlers.add_handler(patterns, callback)

Evidence

The new patterns = ... else url_patterns forwards url_patterns into the high-level registry.
That registry explicitly allows raw dict patterns for the browser intercept, but _HandlerEntry
only builds match regexes from string patterns and returns True when no regexes exist, and
_on_event calls callbacks before checking isBlocked, so dict-only patterns can make callbacks
run for unrelated requests.

py/private/bidi_enhancements_manifest.py[1175-1180]
py/private/_network_handlers.py[246-264]
py/private/_network_handlers.py[624-636]
py/private/_network_handlers.py[714-726]

Agent prompt

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

## Issue description
High-level request handlers can now receive `url_patterns` via the `callback=` keyword path. If callers pass wire-level dict UrlPatterns (which the codebase explicitly allows for browser-side filtering), Python-side matching treats them as “no patterns” and `matches()` becomes unconditional, so callbacks may run for unrelated (unblocked) requests.

## Issue Context
- The PR change forwards `url_patterns` when `event is None`, enabling dict patterns to flow into the high-level registry.
- The high-level registry executes callbacks before checking `isBlocked`, so overly-broad `matches()` affects unblocked requests too.

## Fix Focus Areas
- py/private/bidi_enhancements_manifest.py[1175-1180]
- py/private/_network_handlers.py[246-264]
- py/private/_network_handlers.py[624-636]
- py/private/_network_handlers.py[714-726]

## Suggested fix
Implement Python-side URL matching for dict UrlPatterns (e.g., parse URL and compare provided keys like protocol/hostname/port/pathname), OR explicitly reject dict patterns in the high-level handler API with a clear error directing users to the legacy phase-based API when they want wire-level UrlPatterns.

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

---

Previous review results

Review updated until commit a2f5b52

Results up to commit f999fc6

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

[qodo-code-review]

PR Summary by Qodo

Fix BiDi Network.add_request_handler callback= path to honor url_patterns
🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Walkthroughs

Description

• Ensure callback= request handlers fall back to url_patterns when event is omitted.
• Prevent unscoped intercepts that previously matched every request.
• Add regression test asserting urlPatterns are sent to network.addIntercept.

Diagram

graph TD
  A["Caller code"] --> B["Network.add_request_handler"] --> C["Dispatch: patterns = event ?? url_patterns"] --> D["RequestHandlers.add_handler"] --> E["BiDi: network.addIntercept"] --> F["Browser"]

Loading

High-Level Assessment

The chosen fix is the simplest and most consistent approach: it makes the callback= (keyword) dispatch path behave like the positional path by ensuring patterns are derived from url_patterns when event is None. Considered alternatives (e.g., signature/overload restructuring) would be more invasive for minimal additional value.

File Changes

Bug fix (1)

bidi_enhancements_manifest.py Fallback to url_patterns when callback= is used without an event +2/-1

Fallback to url_patterns when callback= is used without an event

• Fixes the callable(callback) dispatch branch so that when event is None (common with callback= keyword usage), the handler registration uses url_patterns instead of passing None. This prevents accidentally creating a global intercept that matches every request.

py/private/bidi_enhancements_manifest.py

---

Tests (1)

bidi_network_tests.py Add regression test for callback= + url_patterns scoping +13/-0

Add regression test for callback= + url_patterns scoping

• Adds a unit test asserting that network.add_request_handler(callback=..., url_patterns=...) results in network.addIntercept receiving the translated/scoped urlPatterns rather than an unscoped intercept.

py/test/unit/selenium/webdriver/common/bidi_network_tests.py

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit a2f5b52