The #3549 test only covered LP.getSemanticTree's text format. Cover
the JSON format with interactiveOnly on the same fixture, and drive the
MCP tree and nodeDetails tools through call() with a child-frame
backendNodeId. Passing the main frame in either tool now fails the
suite instead of only tripping a debug assert.
A nodeId root inside a child frame was walked with the main frame. The
style checks already resolve the owner frame per element, but the
listener map was built from the main frame's event manager, so
listener-only elements in the iframe were reported non-interactive, and
relative hrefs resolved against the parent's base URL.
Resolve the root's owner frame before the walk, as getSemanticTree and
getNodeDetails do.
Applies the frame-ownership pass to SemanticTree, copying what we did for
StyleManager (1). SemanticTree doesn't visit iframes, so the frame of the root
is the frame/frame._style_manager we need to target for all visited nodes.
Like #3536, it's up to the callers to (a) get the correct frame and (b) decide
what to do on a frameless-node.
(1) https://github.com/lightpanda-io/browser/pull/3536
The two low-level trusted-event dispatchers took button (which button changed)
and buttons (the held mask) as adjacent, swappable positional args. Bundle the
shared event fields into a PointerInput struct with named fields, mirroring the
file's HoverContext, so a transposition is a field name rather than a silent bug.
Fold the two loose Page fields (input_pressed_buttons, input_mousedown_suppressed)
into a PointerButtons struct in user_input.zig that owns the chord mask and the
press/release transitions, so triggerMousePress/Release keep only dispatch.
Add runMouseDownFocus so the three click callers stop repeating the
!suppress_mouse and !suppress_focus gate; a focus-error policy param keeps each
caller's warn-vs-propagate behavior.
Trim the verbose dispatch/trigger and CDP-test comments to a single sentence
each, moving behavior contracts onto the function doc-comments; also drop the
stale buttonsMask comment orphaned by the earlier buttonsBitmask reuse.
Round-1 review feedback from arrufat on #3507 (share pointer/mouse click
dispatch across click paths):
- CDP mousePressed now threads clickCount into mousedown's detail
(mouse_detail), matching the MCP path (actions.click) instead of
always firing detail 0. Threaded through BiDi's press call too, which
was already tracking click_count for release but silently dropping it
on press.
- dispatchClickAsPointer now carries the still-held buttons mask instead
of hardcoding 0, so a primary click firing mid-chord (the primary
button releasing while another button is still held) reports the
correct PointerEvent.buttons.
- Fixed a regression caught while verifying the above against live
Chrome: the first fix's fallback forced clickCount 0 (CDP's default
when the field is omitted) to detail 1. Chrome and Firefox both
preserve 0 there, and Chrome doesn't fire `click` at all in that case
— the fallback now preserves 0 instead of forcing 1.
Left two pre-existing issues alone, flagged in code comments instead of
fixed, per arrufat's own scoping:
- triggerMouseRelease's parallel clickCount-0 fallback has the same
mismatch on the release side; not introduced by this PR.
- The chord's activation-event ordering (contextmenu on the right press
vs. this codebase's release-only contextmenu, and no auxclick) is a
real, pre-existing deviation from both Chrome and Firefox, confirmed
on Firefox too during this round, not Chrome-specific.
Test coverage: a parametrized CDP test over clickCount 0/1/2 asserting
mousedown's detail; a chord test asserting a mid-chord primary click's
buttons mask. Two more tests were added independently during review
audit and kept as-is: clickCount 2 on a press+release pair asserting
detail 2 on mousedown/mouseup/click plus dblclick, and a chorded press
asserting its own mousedown branch also carries the press message's
clickCount.
Confirmed against live Chrome (headless, raw CDP) and Firefox (headless,
WebDriver BiDi) for both the mousedown-detail and chord-buttons fixes.
zig fmt --check clean; cdp.input 20/20 and bidi.input 5/5 pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A second independent Codex audit on the rebased bdabfea42 found a real
regression the chord fix itself introduced: the chorded-press branch in
triggerMousePress dispatched the compatibility mousedown but discarded its
cancellation result and never called focusForMouseDown, unlike the
first-button path. Pressing a second button on a different element while
the first is still held (e.g. right-click a second field while holding
left on the first) left focus on the original element instead of moving
it to the new mousedown's target, diverging from real Chrome.
Independently verified before committing:
- Traced the diff: the chorded branch's `_ = try dispatchMouseEventOn(...)`
discarded the return value entirely, so suppress_focus was never
computed and focusForMouseDown was never reachable from that branch —
confirmed this matches the first-button path's own
`if (!press.suppress_mouse and !press.suppress_focus) try
focusForMouseDown(...)` structure, which the chord branch should mirror
but didn't.
- Reverted the one-line fix (kept the new test staged) and confirmed the
new "chorded mousedown focuses its target unless pointerdown or
mousedown was cancelled" test fails against the pre-fix code, then
restored it.
- Ran the full suite: zig build test and -Dwpt_extensions both 1521/1521.
- Rebuilt the binary and ran the extended tools/shared-click-audit.mjs
--assert-chord --assert-chord-focus against it: chord_focus_contract
PASS, with the raw event trace confirming focus actually landed on the
second element ("ticket").
- Ran the same harness with --chrome: chord_focus_contract PASS against
real Chrome too, independently confirming this is the correct target
behavior and not just the audit's claim.
Fix mirrors the existing first-button path exactly: capture the chorded
mousedown's own cancellation result and run focusForMouseDown when it
wasn't cancelled, still gated by the gesture-level suppression flag so a
cancelled initiating pointerdown still suppresses every mousedown in the
chord, as before.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XLXnBHBxQNskg2MAke3Lhv
An independent audit of the shared click-dispatch refactor (Codex, on
b2cd5a190) reproduced a real defect: triggerMousePress/triggerMouseRelease
treated every button press/release as its own complete gesture, with no
notion of another button already held. Pressing a second button while the
first was still down (a chord) fired a second pointerdown with the wrong
buttons mask (single-button bitmask, not the aggregate), and
Page.input_mousedown_suppressed — a bare bool — got overwritten by the
second press's own suppression outcome, discarding whether the gesture's
initiating pointerdown had actually been cancelled. The eventual release of
the first (cancelled) button then incorrectly fired mouseup. Verified
against real Chrome 152's behavior for the identical input (one pointerdown,
button changes as pointermove, one final pointerup, no compatibility
mousedown/mouseup once the initiating pointerdown is cancelled) per
https://www.w3.org/TR/pointerevents3/#chorded-button-interactions.
Fixed by adding Page.input_pressed_buttons (an aggregate mask) and
dispatching pointerdown/pointerup only at its 0/nonzero transitions;
a button change while another remains held fires pointermove instead, and
input_mousedown_suppressed is now set once at gesture start and held for
the whole chord rather than being overwritten per press. Scoped to
triggerMousePress/triggerMouseRelease only — dispatchPointerPress/Release
(used by actions.click and WebDriver.click, which can't chord) are
unchanged. BiDi's input path calls the same two functions, so it gets the
fix for free.
One wrinkle caught while fixing: an existing test dispatches two
mousePressed calls on the *same* button with no release between them (to
test focus in isolation, not a real chord). Keyed the fresh-gesture check
off "some *other* button already held" rather than "any button held" so
that pattern still starts a fresh gesture, matching its own expectation.
New test: "a mouse chord fires pointermove for the mid-gesture button
change, not a second pointerdown/pointerup" on a new #btnChord fixture
element (mcp_actions.html). Confirmed it fails against the pre-fix code
(TestUnexpectedResult, verified by stashing the fix, running the test, and
restoring) before trusting it.
Verification: zig build test and zig build test -Dwpt_extensions:
1517/1517 both ways. zig fmt --check and git diff --check clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XLXnBHBxQNskg2MAke3Lhv
An automated review pass (Grok) on the shared click-dispatch diff flagged
comments that narrated the refactor's history ("for the first time",
"deliberately-untouched", "shares its dispatch mechanics with...") instead
of stating the invariant a reader needs. Rewrote four: WebDriver.click's
never-fails contract, actions.click's focus-error policy, the mousedown
click-count comment (also fixed to attribute the gap to triggerMousePress's
signature, not a CDP-only quirk — bidi/input.zig hits the same gap), and
the two new CDP regression tests' descriptions.
One suggested fix (clear input_mousedown_suppressed defensively before
dispatchPointerPress) was reviewed and rejected: suppress_mouse=true is a
constant once computed, with no fallible call between that assignment and
return, so the flag can only be lost on a throw when suppress_mouse=false —
which is the value already held by design. The scenario Grok describes
additionally requires an unpaired mousePressed (protocol misuse), not a
leak in this code.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XLXnBHBxQNskg2MAke3Lhv
Follows up on the maintainer's review note on PR #3431: WebDriver.click
(testdriver), Frame.user_input.triggerMousePress/Release (CDP's
Input.dispatchMouseEvent, i.e. Puppeteer/Playwright), and actions.click
(MCP) each dispatched their own near-identical pointerdown/mousedown/
pointerup/mouseup/click sequence. Adds three shared functions to
frame/user_input.zig -- dispatchPointerPress, dispatchPointerRelease,
dispatchClickAsPointer -- and routes all three call sites through them.
This gives the CDP path pointerdown/pointerup for the first time (it
previously only fired bare mousedown/mouseup/click), and a PointerEvent
click (previously a plain MouseEvent there). It also gives WebDriver.click
suppress/focus handling it never had: that function used to dispatch its
fixed five-event sequence unconditionally, ignoring preventDefault() and
never moving focus.
Because CDP's mousePressed and mouseReleased arrive as two independent
Input.dispatchMouseEvent messages with no shared call stack, a new
Page.input_mousedown_suppressed field carries whether the press half's
cancelled pointerdown should suppress this gesture's mouseup on the
release half. It's read-and-reset unconditionally at the top of
triggerMouseRelease (and reset on a press that finds no element), so an
unmatched or missed message can't leak stale state into the next gesture.
dispatchPointerPress returns PressResult{suppress_mouse, suppress_focus}
rather than running the focus default action itself: focusForMouseDown
can fail, and the three callers don't agree on what that should mean for
the click (actions.click: warn and continue; WebDriver.click and CDP:
propagate), so each runs it against the result with its own handling.
WebDriver.click also now reads frame._page.input_modifiers so a held
modifier key still reaches its dispatched events, matching its own
pre-existing local helpers' behavior (only compiled under
-Dwpt_extensions; actions.click and CDP don't track modifier state, so
they pass an empty Modifiers{}).
WebDriver.actionSequence's performPointerSource (a fourth, more complex
copy -- click counts, drag chords, touch) is deliberately left alone, as
is its own pre-existing gap (no pointerdown-suppresses-mousedown there).
Two new CDP tests reuse the existing mcp_actions.html fixture (#btn
records the full event sequence; #btnPreventDefault's pointerdown
listener calls preventDefault()) to pin the new pointer events and the
cross-message suppression. Both were confirmed to fail against the
pre-refactor triggerMousePress/Release bodies. Extended #btn's recorder
with event.detail after an independent review caught mousedown/mouseup's
click count silently dropping to 0 at all three call sites in an earlier
version of this change; the MCP click test's assertion was updated to
match.
zig build test and zig build test -Dwpt_extensions: 1516/1516 both ways.
zig fmt --check clean on all seven changed files.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XLXnBHBxQNskg2MAke3Lhv
Follow up to a chain of iframe context correctness (3520, 3510, 3501). Every
caller of StyleManager now must make sure the they call use the correct
StyleManager for a given Element. The StyleManager enforces this correctness
with a debug-only assertion.
It's tempting to think that this could be handled internally by the
StyleManager. It would be a lot cleaner..it can do the element -> ownerFrame
lookup. The issue is with frameless elements/documents which the StyleManager
cannot handle: every caller needs to decide how to handle this case.
The overflow shorthand split lives once, in the CSS parser, for both the
style declaration and the StyleManager attribute fold; the two copies
disagreed on three-value input. The declaration states the longhand-pair
rule once and serializes the pair where its first longhand sits without
a second scan. The two listener scans merge into one with a filter. The
per-axis scroll walk goes: the cascade memoizes each element, so calling
the single-axis walk twice costs a few lookups.
setProperty updated an existing entry in place, so on
style="overflow: hidden; overflow-y: scroll" a later
setProperty("overflow", "hidden") left overflow-y winning, and
removeProperty("overflow") left it behind. Now that the cascade folds
overflow, that showed up as a stale scroll container.
The declaration object stores overflow the way the CSSOM does for every
shorthand: set and parse expand it into overflow-x and overflow-y,
remove drops both, and reading or serializing recombines them when both
are present with the same priority. It is the one shorthand handled this
way because it is the one whose longhands StyleManager tracks.
Blink does not dispatch a second event for the legacy name. Per target,
it runs the wheel listeners if there are any, else the mousewheel ones
with the event retyped for the call. A target registering both never
sees mousewheel. The event manager now does the same for trusted
events, so one wheel dispatch covers both names and the fallback also
decides what counts as a non-passive listener on the path.
hasNonPassiveListener walked parentNode up to the window, which is not
the path an event takes: it missed shadow hosts, composed retargeting
and inline handlers. The wheel and touch dispatchers now flag the event
as cancelable-unless-passive and dispatchNode resolves the flag against
the path it just built.
With six tracked properties an element declaring all of them inline is
not a case worth a branch; checkRules already skips rules nothing weaker
than inline could beat.
ScrollTarget and ScrollTargets are only named inside Element.zig; callers
switch on the result or call scrollBy through it. wheelScroll has one
caller, wheel, in the same file.
zenai now ships openrouter and orcarouter presets (OPENROUTER_API_KEY /
ORCAROUTER_API_KEY, provider-prefixed model ids). The provider enum is
derived from zenai, so only the help text, key hint, and README need
to name them.
This adds the shell for ServiceWorker, behind
`--experimental-features serviceworker`.
It's pretty useless as-is. We don't have the CacheStorage API (next) and don't
have the fetch interceptor (next next). But as-is, the change is quite big but
thankfully largely isolated.
Add state to OpenContext so that the owner is tracked known and other flows
don't free (cancelPark doesn't free when the context is running, since run will
take care of it)
A name is a substring or a regex, never both, so the filter says so
instead of carrying two optionals the caller has to null against each
other. Whether `/.../x` is a literal is now decided by JavaScript's flag
alphabet rather than "looks like letters", which stops `/usr/bin` from
being read as a pattern with flags. Comments that narrated callers or
the type name are gone; the literal parser gets its own test in place
of two MCP round-trips.
A model that writes `/spice/i` should not be told the name has a
stray `i` in it. `i` and `u` are already how names match, `s` and `m`
map to their PCRE2 options, and any other letter is a tool error that
names it. Text after the closing slash that is not a letter makes the
whole thing a plain substring again.
Adrià Arrufat