Skip to content

fix(browser): rank dialog and viewport elements before truncating (#2) - #8

Closed
Enard52 wants to merge 1 commit into
thethirdsourcers:mainfrom
Enard52:fix/dialog-element-ranking
Closed

Enard52 wants to merge 1 commit into
thethirdsourcers:mainfrom
Enard52:fix/dialog-element-ranking

Conversation

@Enard52

@Enard52 Enard52 commented Oct 14, 2026

Copy link
Copy Markdown

Summary

Closes #2

On busy pages with large navigation bars or complex layouts (e.g. cloud dashboards), browser_elements previously truncated elements at 150 strictly in document order. Consequently, modal dialog fields and buttons appearing after 150 navigation links were never returned.

This PR ranks elements before truncation:

  1. Tier 0: Elements inside an open <dialog open> or [role=dialog] / [role=alertdialog].
  2. Tier 1: Elements visible in the current viewport (r.bottom > 0 && r.top < innerHeight && r.right > 0 && r.left < innerWidth).
  3. Tier 2: Visible elements rendered outside the current viewport.

Document order is preserved within each tier.

Changes

  • src/browser.ts: Updated ELEMENTS_JS to compute element ranking and sort candidate controls stably before assigning IDs (e1, e2, ...) and truncating to 150 items.
  • tests/browser.test.ts: Added comprehensive unit test coverage:
    • Verifies ranking precedence: dialog controls first, then viewport elements, then offscreen elements.
    • Verifies preservation of modal dialog controls even when preceded by >150 sidebar items in document order.

Test Plan

  • Run npm test: all 175 tests pass (0 failures).

@Enard52

Enard52 commented Oct 21, 2026

Copy link
Copy Markdown
Author

Closing this as superseded rather than rebasing it — there is no union left to keep.

main now ranks before cutting to 150, in commit b06c2c4 ("Browser tools round 7: single-page apps, ranking before the 150 cap, search boxes"), and that implementation covers the same three tiers this PR introduced and more:

  • src/browser.ts builds const rank = el => { if (inPopup(el) || popupRole(el)) return 0; const r = __rect(el); return r.top + r.height > 0 && r.top < innerHeight && r.left < innerWidth && r.left + r.width > 0 ? 1 : 2 }, then ranked() sorts by a.r - b.r || a.i - b.i (rank, then page order — the same stable tie-break this PR used) before order.slice(0, 150).
  • Its tier 0 is broader than mine: dialog[open], [role=dialog], [role=alertdialog], [aria-modal=true] (taking the topmost with .pop()), plus [role=menu], [role=listbox] containers, open menu/option roles via popupRole, and elements whose shadow host is inside a popup. Mine only matched dialog[open] / [role=dialog] / [role=alertdialog] through closest().
  • Coordinates come from __rect(), which adds ancestor frame offsets, so the viewport tier works inside iframes — this PR used the raw getBoundingClientRect().

That is why the branch reports CONFLICTING instead of needing a mechanical rebase: both sides rewrote the same block of src/browser.ts (and the matching tests) to do the same thing, and main's version is the better one.

Two things that are still open, and I'd rather raise them honestly than smuggle them into a dead branch:

  1. browser_elements stops at 150 elements, so dialogs on busy pages are unreachable #2 itself is still open although b06c2c4 fixed it — someone with permissions can probably close it.
  2. The ranking has no regression test. tests/browser.test.ts checks page scripts only for shape (ELEMENTS_JS.includes(...) for shortHrefFor, and that every script compiles with new Function) — nothing asserts that a control inside an open dialog survives the 150 cap when 160 sidebar links precede it in document order, which is exactly the failure mode browser_elements stops at 150 elements, so dialogs on busy pages are unreachable #2 described. I'd like to send that as a tests-only PR against current main; if it's wanted, say so here or on browser_elements stops at 150 elements, so dialogs on busy pages are unreachable #2 and I'll open it, mock DOM and all.

Thanks for the work on the browser tools rounds — the direction in b06c2c4 matched what I'd have wanted next.

@Enard52

Enard52 commented Oct 21, 2026

Copy link
Copy Markdown
Author

Superseded by the ranking in main's b06c2c4 — comparison in the comment above.

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.

browser_elements stops at 150 elements, so dialogs on busy pages are unreachable

1 participant