Skip to content

fix(ui): give the wheel back after a <select> is used with the mouse - #147

Merged
stormer78 merged 1 commit into
mainfrom
fix/select-wheel-scroll-lock
Aug 31, 2026
Merged

fix(ui): give the wheel back after a <select> is used with the mouse#147
stormer78 merged 1 commit into
mainfrom
fix/select-wheel-scroll-lock

Conversation

@stormer78

Copy link
Copy Markdown
Contributor

The report

I can't scroll down in the setup window. The Save button is out of the viewport and I can't get to it. When I select the DID self-issued option under Secret kind, I get locked from scrolling. But when I click outside of it, the lock is released.

Reported on Vivaldi/macOS, but nothing here is Vivaldi-specific — it is Chromium.

The cause

Chromium routes wheel events to a focused menulist <select> and does not chain them on to the document. Once an option is picked, the wheel is dead for as long as the pointer stays over that control. Page Up/Down goes to the select as well, so the keyboard route out is closed too.

Confirmed by the reporter, not inferred: scrolling with the cursor parked away from the control works; Page Up/Down moved the selection instead of the page.

In the popup — viewport capped near 600px — the Secret kind control sits directly over the fields still to be reached, so it reads as the whole window having seized up. The only way out is a click on something uninteresting.

The fix

releaseSelectAfterPointerChange (src/select-wheel.ts) performs that click's one useful effect on the same gesture: it blurs the <select> the moment a pointer-driven choice lands. Installed on the three surfaces that render one — popup, options, confirm. Not popup-specific; the popup is only where the height cap makes it fatal rather than irritating.

The pointer gate is the load-bearing part. Blurring on every change would trade a mouse annoyance for a keyboard trap: a closed <select> driven by the arrow keys fires change on every keystroke, and a blur there ejects focus mid-selection and resets tab position to the top of the document. A keydown cancels the pending release, which also disarms a menu that was opened and then dismissed.

Typed against a structural SelectWheelHost rather than Document: HTMLSelectElement does not exist in Node and the extension's tests run with no DOM.

Also here

The new-vault-entry form pins its Save/Cancel to the bottom of the scrollport. This was written before the cause was known and aimed at the wrong one, but it stands on its own merits: a did-self-issued entry is eight fields tall against a 600px viewport, and a form whose last step is only ever reachable by scrolling is a defect independent of whether the scrolling works. Sticky only engages once the form overflows, so the options page is unchanged when it fits.

Verification

Lint, build and 673 tests pass. Seven new tests pin both halves of the pointer/keyboard split, the abandoned-menu case, target discrimination, and disposal.

Browser-level confirmation is still outstanding and belongs to a human: automation cannot drive an extension popup bubble, and a tab-hosted popup.html is not the same context. To check: reload unpacked, open the popup, pick did-self-issued, and the wheel should work immediately with the cursor still over the Secret kind box.

Pre-merge checklist

- [x] No new reqwest::Client::new() / bare fetch(); all clients have finite timeouts (R1.2) — no network code touched
- [x] No lock held across a network await (R1.3) — n/a
- [x] No local state committed before its remote effect, or the flow is resumable with an idempotency key (R2.1) — n/a
- [x] Every retry is bounded + backed off; non-idempotent ops are not blind-retried (R1.4) — n/a
- [x] Accept/poll/listen loops survive transient errors (R1.5) — n/a
- [x] Acks/deletes happen only after durable handoff (R1.6) — inbound path untouched
- [x] New/changed wire types: camelCase, deny_unknown_fields where security-relevant, schema registered, all consumers (incl. JS) updated (R3.*) — no wire types
- [x] Config absence = most restrictive; fail-closed if enforcement can't start (R5.*) — n/a
- [x] Logs/status claim only what was verified; background-job failures are surfaced (R6.*) — n/a
- [x] "Process dies on the next line" answered for every mutation touched (R2.1) — no mutations
- [x] Deviations from this guide flagged explicitly with rule numbers — none

UI-only change confined to packages/extension.

Chromium routes wheel events to a focused menulist <select> and does not
chain them on to the document, so once an option is picked the wheel is
dead for as long as the pointer stays over that control. Page Up/Down
goes to the select too, so the keyboard route out is closed as well. In
the popup, whose viewport Chrome caps near 600px, the control sits
directly over the fields still to be reached — it reads as the whole
window having seized up, and the only way out is a click somewhere
uninteresting.

`releaseSelectAfterPointerChange` performs that click's one useful
effect on the same gesture: it blurs the <select> the moment a
pointer-driven choice lands. Installed on the three surfaces that render
one — popup, options and confirm. This is not popup-specific; the popup
is only where the height cap makes it fatal rather than irritating.

The pointer gate is the load-bearing part. Blurring on every `change`
would trade a mouse annoyance for a keyboard trap: a closed <select>
driven by the arrow keys fires `change` on every keystroke, and a blur
there ejects focus mid-selection and resets tab position to the top of
the document. A `keydown` therefore cancels the pending release, which
also disarms a menu that was opened and then dismissed.

The module is typed against a structural `SelectWheelHost` rather than
`Document` because `HTMLSelectElement` does not exist in Node and the
extension's tests run with no DOM.

Separately, the new-vault-entry form pins its Save/Cancel to the bottom
of the scrollport. That was aimed at this bug before the cause was
known, and stands on its own merits: a did-self-issued entry is eight
fields tall against a 600px viewport, and a form whose last step is only
ever reachable by scrolling is a defect independent of whether the
scrolling works.

Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
@stormer78
stormer78 merged commit 07bd96d into main Aug 31, 2026
3 checks passed
@stormer78
stormer78 deleted the fix/select-wheel-scroll-lock branch August 31, 2026 05:48
@affinidi-appsecurity-bot

affinidi-appsecurity-bot commented Aug 31, 2026

Copy link
Copy Markdown

🛡️ AI Agentic Security Code Review

1 finding needs a human to review/validate.

Mandatory to check: 🔒 Security Code Review Report

Details

🛡️ Security Code Review Report — PR #147

Field Value
Repository OpenVTC/vta-browser-plugin
Branch fix/select-wheel-scroll-lockmain
Validated 2026-09-05
Scan ID 7322123a
Validator AI Security Validation Agent

🗺️ Scan Coverage

Modules scanned: 1 · with findings: 1 · files: 6 · findings: 2

Module Files scanned Findings
packages/extension 6 2

Executive Summary

Category Confirmed Must-Review-By-Human
Security Issues 0 1

⚠️ 1 finding(s) need human review. These could not be conclusively confirmed or dismissed automatically (insufficient evidence). They are not dismissed — a developer / security team member must read and decide.


🔒 Security Issues

⚠️ Must-Review-By-Human (1)

Validated up to a point, but inconclusive — a human must read the code and make the final call. Reported (not dismissed) so developers and the security team receive them.

⚪ Unbounded, non-disposed capture-phase DOM event listeners registered on module evaluation (resource leak / no cleanup)

Field Detail
Severity INFORMATIONAL
Location packages/extension/src/confirm.tsx:1332
Finding ID github_pr-7bf0196aa769
CWE CWE-401 Missing Release of Memory after Effective Lifetime, CWE-1050 Excessive Platform Resource Consumption within a Loop
OWASP A05:2021 Security Misconfiguration
MITRE ATT&CK T1499 Endpoint Denial of Service (weakly related, defense-in-depth note only)
CAPEC CAPEC-130 Excessive Allocation
DREAD 1.8
Reachability 🔴 Reachable
Exploit Maturity theoretical
Detection Source skill_scan

Summary: The wheel-scroll fix registers document-level capture-phase event listeners in three extension entry scripts but discards the disposer function each call returns, meaning the listeners are never removed even though the code was explicitly designed to support cleanup.

📝 Description:

In the normal browser-extension lifecycle (fresh document per popup/options open) this has zero observable impact. Under dev-mode HMR or a future refactor that re-invokes this module without a full document reload, users could experience progressively duplicated blur-on-change behavior and minor listener-driven CPU overhead in the popup/options/confirm UI.

🧪 Proof of Concept:

releaseSelectAfterPointerChange(document) returns a disposer function (see select-wheel.ts) that is never captured or called here, so if this module-level statement is ever executed more than once against the same document (e.g. bundler HMR), duplicate listeners accumulate with no mechanism to remove earlier registrations.

// Chromium leaves the wheel pointed at a focused <select>; without this,
// picking an option kills scrolling over that control until the user
// clicks elsewhere. See src/select-wheel.ts.
releaseSelectAfterPointerChange(document);

const root = document.getElementById("root");
if (root) {
  createRoot(root).render(

Vulnerable lines: 991, 996

🔁 Reproduction Steps:

  1. Build the extension in dev mode with a bundler that supports HMR for popup.tsx/options.tsx/confirm.tsx.
  2. Open the popup and trigger a hot-reload of popup.tsx without a full page navigation.
  3. Observe (via a debugger or event-listener counting instrumentation) that the pointerdown/keydown/change listener count on document increases with each reload rather than staying constant at 3.

🔎 Evidence: packages/extension/src/confirm.tsx:1332

releaseSelectAfterPointerChange(document);

const root = document.getElementById("root");

💥 Impact:

In the normal browser-extension lifecycle (fresh document per popup/options open) this has zero observable impact. Under dev-mode HMR or a future refactor that re-invokes this module without a full document reload, users could experience progressively duplicated blur-on-change behavior and minor listener-driven CPU overhead in the popup/options/confirm UI.

Confidentiality: none · Integrity: none · Availability: low - only exploitable under repeated re-evaluation of the same document (e.g. HMR, hot-reload workflows, or future refactors introducing re-invocation); standard single popup/options open-close lifecycle is unaffected since each page gets a fresh document context on open in a browser extension.

🧭 Reachability:

  • Network exposure: none
  • Auth barrier: none
  • Attack path: EP-001/EP-002/EP-003 (extension page load) → module top-level call to releaseSelectAfterPointerChange(document) at confirm.tsx:1332 / options.tsx / popup.tsx → select-wheel.ts#releaseSelectAfterPointerChange registers 3 undisposed listeners

⚖️ Triage Factors:

Factor Value
Fixable ✅ Yes
Exploitability low
Business impact low
Public exploit None known
Environment unknown

Attack scenario: A future code change or dev-mode hot-reload that re-evaluates the same popup/options/confirm script on an already-live document would stack additional identical capture-phase listeners with no way to remove earlier ones, since the disposer returned by releaseSelectAfterPointerChange is discarded.

🔧 Remediation:

⚠️ AI-generated fix. Review, test in staging, and validate against your architecture before applying.

Storing the disposer and wiring it to the page's unload event ensures listeners are always cleaned up even if the module is ever re-evaluated (dev HMR, future code paths), removing the reliance on 'this page never reloads its own script' as an implicit invariant.

Vulnerable code:

releaseSelectAfterPointerChange(document);

const root = document.getElementById("root");

Secure code:

const disposeSelectWheelFix = releaseSelectAfterPointerChange(document);
window.addEventListener("unload", disposeSelectWheelFix, { once: true });

const root = document.getElementById("root");

Additional recommendations:

  • Add a lint rule / code comment enforcing that any addEventListener call at module scope must have a corresponding disposal strategy.
  • Consider a WeakMap-guarded singleton so calling releaseSelectAfterPointerChange(document) twice on the same host is a no-op.

🔍 Validation Log

  • Verdict: ⚠️ Must-Review-By-Human
  • Confidence: 90%
  • AI Validation Evidence: EVIDENCE FOUND: confirm.tsx calls releaseSelectAfterPointerChange(document); at module top-level (line 1332), immediately before const root = document.getElementById("root"); and the createRoot(root).render(...) call. The comment above it explains: 'Chromium leaves the wheel pointed at a focused ; without this, picking an option kills scrolling over that control until the user clicks elsewhere.' This confirms the listener-registration call exists and is invoked unconditionally at m Validation Effort: This finding was validated up to a point, but the available evidence was insufficient for a conclusive automated verdict. A human (developer / security team) must manually review the code and decide. Not dismissed — treat as an open item pending human review. Generated by Agentic Sec — AI Security Validation Agent This report includes full scan data + AI validation evidence. Feed to engineering copilots for automated fix deployment. Complementary: 🛡️ **Threat Model & Affect Analysis** 🛡️ Threat Model & Affect Analysis — PR fix(ui): give the wheel back after a <select> is used with the mouse #147 Field Value Repository OpenVTC/vta-browser-plugin Branch fix/select-wheel-scroll-lock → main Generated 2026-09-05 ℹ️ This report contains theoretical threats and impact analysis for the MR. Unlike the Security Code Review Report (which contains confirmed, materialised issues), these are potential risks that may or may not be exploitable. Use this for defence-in-depth planning. 📋 Affect Analysis Change Summary This PR fixes a Chromium UX bug where the mouse wheel becomes unresponsive over a element after it is used with the pointer, by adding a new shared utility (select-wheel.ts) that blurs the select on change after a pointer-driven open, wired into three extension entry pages (confirm, options, popup). It also makes the vault entry form's action bar sticky so the Save/Cancel controls remain reachable when the popup viewport is short. No security-sensitive data handling, authentication, or authorization logic is touched.

    Diff: +218 / -2 lines
    Types: bugfix, ui, test

    ⚠️ Security Implications

    🔵 New global capture-phase DOM listeners added to three extension pages including the highest-sensitivity popup page

    New global capture-phase DOM listeners added to three extension pages including the highest-sensitivity popup page

    Action: No change required for this PR as written — the implementation is narrowly scoped and safe. Recommend a lightweight guardrail (code comment + review checklist item) stating that select-wheel.ts must remain limited to tagName/blur() operations and must not be extended to read or log event payload dat

    ⚪ Deliberate pointer-vs-keyboard branching avoids a keyboard focus-trap regression

    Deliberate pointer-vs-keyboard branching avoids a keyboard focus-trap regression

    Action: No action needed; commend this as a good practice. Ensure future modifications to this file preserve the keyboard-cancellation branch via the existing test suite.

    🧩 Affected Components

    Component Impact Change What Changed
    select-wheel utility (COMP-001) low new A new shared DOM utility was introduced that installs global capture-phase pointerdown/keydown/change listeners to fix a Chromium wheel-scro
    confirm-page, options-page, popup-page (COMP-002/003/004) low modified Each of the three extension entry-point scripts now imports and invokes releaseSelectAfterPointerChange(document) once at module load, befor
    vault-panel AddEntryForm (COMP-005) low modified The Save/Cancel action bar for the vault entry form was converted from a static, in-flow flex container to a sticky-positioned bar pinned to

    📁 File Classifications

    packages/extension/src/select-wheel.ts

    • Type: ui

    packages/extension/src/confirm.tsx

    • Type: business-logic

    packages/extension/src/options.tsx

    • Type: business-logic

    packages/extension/src/popup.tsx

    • Type: security

    packages/extension/src/vault-panel.tsx

    • Type: security

    packages/extension/tests/select-wheel.test.mts

    • Type: test

    🛡️ STRIDE Threat Model

    No STRIDE threats identified for this MR.


    🍝 PASTA Threat Model

    Technical Scope

    Entry Points (5): EP-001 DOM_RENDER confirm.html (extension page) -> confirm.tsx#TaskConsent · EP-002 DOM_RENDER options.html (extension page) -> options.tsx#VaultPane · EP-003 DOM_RENDER popup.html (browser action) -> popup.tsx · EP-004 DOM_EVENT document pointerdown/keydown/change (capture) -> select-wheel.ts#releaseSelectAfterPointerChange · EP-005 EXT_MESSAGE options.tsx -> sendToBackground() -> background script

    No PASTA analysis available for this MR.


    Generated by Agentic Sec — Threat Model & Affect Analysis Agent

📊 Summary & findings
✅ Confirmed ⚠️ Must-Review-By-Human
0 1

Must-Review-By-Human (1)

  • Unbounded, non-disposed capture-phase DOM event listeners registered on module evaluation (resource leak / no cleanup)

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.

2 participants