Skip to content

fix(js): honor the click in progress flag - #729

Open
Vrejf wants to merge 1 commit into
h4ckf0r0day:mainfrom
Vrejf:pr/click-in-progress
Open

fix(js): honor the click in progress flag#729
Vrejf wants to merge 1 commit into
h4ckf0r0day:mainfrom
Vrejf:pr/click-in-progress

Conversation

@Vrejf

@Vrejf Vrejf commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #726.

HTMLElement.click() did not implement the HTML click in progress flag, so a handler that clicks its own element re-entered until the V8 stack was exhausted. dispatchEvent wraps every listener in try { ... } catch(e) { console.error(e) }, so the resulting RangeError was logged and did not propagate: the outer click() returned normally and the overflow surfaced only as a console trace.

obscura fetch 'data:text/html,<button id=x>b</button>' --eval "(function(){ var n=0; var x=document.getElementById('x'); x.onclick=function(){ n++; if(n<5000) x.click(); }; x.click(); return n; })()"

Returned 2017 on this host on v0.2.1 and main; the depth is stack-size dependent. Chrome returns 1.

The flag is tracked per element in a closure-private WeakSet and cleared in finally, so it survives a listener that throws and adds nothing page-observable.

Verification

probe Chrome before after
handler clicks its own element 1 to stack exhaustion 1
three sequential clicks 3 3 3
cycle: A clicks B clicks A 2 to stack exhaustion 2
flag clears after a listener throws 2 2 2
handler clicks a different element fires both fires both fires both
  • cargo nextest run --release --features render --no-fail-fast: 1486 run, 1485 passed. The single failure is a timing-sensitive test that also fails on unmodified main.
  • Obstacle course unchanged at 32/33, failing observer-intersection identically on main.

Note on merge order

This is independent of #721 and #722 and applies to main on its own. If it merges alongside #721, the flag check must stay after the disabled early return and before pre-click activation, otherwise a suppressed re-entrant call still flips checked before it returns. That ordering is not observable on main alone, because pre-click activation does not exist yet.

A handler that called click() on its own element recursively dispatched
click events until the stack was exhausted. dispatchEvent catches
listener errors and logs them, so the overflow surfaced only as a console
trace while the outer call returned normally.

Track the flag per element and clear it after click processing, matching
the click() algorithm in HTML. Sequential clicks are unaffected, and a
cycle between two elements stops at the second dispatch because the flag
belongs to the element rather than the call.

Fixes h4ckf0r0day#726.
@Vrejf

Vrejf commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Merge note, since it is only visible from the other side: this and #728 both edit click() and conflict textually. Resolving by taking either side alone silently drops one of two guards that cover different call paths.

Verified on a locally merged tree: the element-level flag from this PR stops a handler that re-enters click() on its own element, while #728's label-level set is the only thing stopping the same bounce over CDP, because a CDP click never calls label.click() and so never puts the label in this PR's set. With both, each path matches headless Chrome (control toggles once, one change, one dispatch); with either alone, one path regresses.

If both are taken, the correct resolution keeps this PR's flag check after the disabled early return and before #728's pre-click activation. Happy to push a pre-resolved branch if that is easier than resolving at merge time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HTMLElement.click() does not honor the click in progress flag

1 participant