morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #1602] [MERGED] fix(browser): report SVG xlink:href links in dom_urls #2998

New issue

Closed

opened 2026-10-17 21:12:43 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:12:43 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1602
Author: @sathariels
Created: 10/10/2026
Status: ✅ Merged
Merged: 10/10/2026
Merged by: @morluto

Base: main ← Head: fix/dom-urls-svg-xlink-href

📝 Commits (2)

2eb66a1 fix(browser): report SVG xlink:href links in dom_urls
cf3edef fix(browser): cache SVG ancestry during DOM capture

📊 Changes

3 files changed (+566 additions, -1 deletions)

View changed files

📝 src/browser/CdpCaptureDocuments.baseUrl.test.ts (+329 -0)
📝 src/browser/CdpCaptureDocuments.ts (+53 -1)
➕ tests/boundary/browser/cdpSvgXlinkHref.test.ts (+184 -0)

📄 Description

fix(browser): report SVG xlink:href links in dom_urls

Summary

inspect-web-page omitted dom_urls entries for SVG links written only as xlink:href. Chromium still follows those links: an SVG <a> with only xlink:href has href.baseVal set and matches :any-link, while an HTML <a xlink:href> does not. This reports the SVG value as href when href itself is missing, so the dom_urls schema stays the same.

Fixes #1586.

Problem and expected behavior

On an SVG <a xlink:href="/xl-s2h"> with no href, domMetadata in src/browser/CdpCaptureDocuments.ts looked up only href, src, action, formaction, and poster. The node was present with attribute_names including xlink:href, and dom_urls had no entry.

Expected: the same entry dom_urls already produces for <a href="/xl-s2h">: attribute href, URL resolved against the document base, and the existing origin policy. When both attributes are present, href wins, including an empty href. An HTML element with only xlink:href stays unreported.

Change and scope

DOMSnapshot.captureSnapshot does not include a namespace URI (confirmed on Chrome 148: document and node keys have no namespace field). SVG context is therefore structural:

the element nodeName is svg, or
an ancestor nodeName is svg, and
a closer ancestor is not foreignObject.

Chrome's HTML parser records those local names as svg and foreignObject. HTML elements are uppercase (A), so an HTML anchor is not SVG context. HTML inside foreignObject stops the walk; a nested svg starts it again. A cyclic or out-of-range parent index fails closed and does not report xlink:href.

Inside that context, a missing href falls back to the exact attribute name xlink:href. The fallback uses the same first-wins attribute map, base URL, and domDestination policy as href, and is stored with attribute "href". browserObservationSchemas.ts is unchanged.

Intentionally not included:

MathML and div href behavior (already reported today; left as-is).
A tag whitelist. rect and other SVG elements that carry xlink:href are reported the same way REA already reports href on elements Chromium does not treat as links. Chrome's href IDL reflects xlink:href for a, image, use, script, feImage, textPath, gradients, pattern, filter, and mpath, and does not for rect.
SVG elements created with createElementNS and inserted outside any svg ancestor. Chrome does treat an orphan SVG <a xlink:href> as a link, but the snapshot cannot distinguish that nodeName a from a MathML or XHTML a.

Contract and boundary impact

Semantic owner and earliest changed stage: domMetadata in the browser CDP capture adapter, while normalizing a DOMSnapshot.captureSnapshot result into metadata.dom_urls. No earlier capture stage changes.
CLI and MCP/tool-catalog contract: none. The dom_urls[].attribute enum stays href | src | action | formaction | poster. Pages that previously omitted SVG xlink:href links now include those URLs as href.
Provider, bridge, target-format, or platform compatibility: Chrome-family CDP DOM snapshots only. No bridge or target-format change. Behavior depends on Chrome's nodeName casing (svg / foreignObject / uppercase HTML).
Evidence, artifact, provenance, or reconstruction contract: none. The added entries are the same derived URL records as existing href values (resolved URL plus destination_scope). Observed attribute names on the DOM node stay xlink:href.
Process execution, authorization, cleanup, or containment impact: none. Destination policy and URL sanitization are unchanged. The new test launches a local headless Chrome only when an executable is already installed.
Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in skill-src/ and never commit build outputs): none. No authored skill, schema, or package metadata change.

Evidence and regression coverage

Tests added or updated:
- src/browser/CdpCaptureDocuments.baseUrl.test.ts: xlink-only SVG <a> (first duplicate wins), href wins in either order, HTML <a> and MathML mrow with only xlink:href stay unlisted, outside-policy origin, SVG image and use resolved against the document base, foreignObject stops SVG context and a nested svg resumes it, empty href beats xlink:href, javascript: is unsupported, cyclic and dangling parent indexes are not SVG.
- tests/boundary/browser/cdpSvgXlinkHref.test.ts: real headless Chrome loads the issue page (plus HTML, image, use, and a foreign origin) and CdpBrowserProvider.inspectPage checks metadata.dom_urls. Skipped when no Chrome-family executable is installed.
Base reproduction or other evidence: executed. On 6d8f95e3 (upstream/main), before the production change, npm run test:focused -- src/browser/CdpCaptureDocuments.baseUrl.test.ts failed 6 tests. Each xlink-only case received urls: [] instead of an href entry. Example: "reports an SVG anchor that only has xlink:href" expected https://app.example.test/xl-s2h and received []. The href-wins, empty-href, and non-SVG cycle cases already passed because no xlink:href fallback existed.
User-visible CLI/MCP output (if applicable): CdpBrowserProvider.inspectPage is the provider behind CLI and MCP. For the issue anchor the entry is:

{
  "node_index": 3,
  "attribute": "href",
  "url": "https://app.example.test/xl-s2h",
  "destination_scope": "approved"
}

The real Chrome run resolved /xl-s2h, /href-wins, /xl-image, and /xl-use on the loopback origin as href / approved, reported the https://cdn.example.test/out link as href / outside_policy with url: null, and left the HTML <a xlink:href="/xl-html"> out of dom_urls. The compiled CLI was not invoked.

Remaining proof gaps:
- An SVG element with no svg ancestor (script-inserted orphan) is not reported.
- xlink:href on SVG elements whose IDL href does not read it, such as rect, is reported.
- Firefox and Safari were not tested.
- npm run verify:browser was not run.
- npm run docs:check was not run; no docs or contracts changed.
- npm run check did not finish verify:architecture-guards because format:check fails on two pre-existing files. That guard was run separately and passed.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm ci (Node v24.18.0, npm 11.16.0) - exit 0, 376 packages installed.
npm run test:focused -- src/browser/CdpCaptureDocuments.baseUrl.test.ts on the base revision, tests only - exit 1. 6 failed, 26 passed (32). Failing cases received [] for xlink-only SVG a, HTML-page SVG a, outside-policy, image/use, nested svg, and javascript: xlink:href.
npm run test:focused -- src/browser/CdpCaptureDocuments.baseUrl.test.ts after the fix - exit 0. 32 passed.
npm run test:focused -- tests/boundary/browser/cdpSvgXlinkHref.test.ts - exit 0. 1 passed (headless Chrome 148.0.7778.96). Repeated after a small assertion tightening; still 1 passed.
npm run test:focused -- src/browser/CdpCaptureDocuments.baseUrl.test.ts tests/boundary/browser/cdpSvgXlinkHref.test.ts - exit 0. 33 passed.
npm run verify:test-discovery - exit 0. Discovered all 766 test files exactly once.
npm run check - exit 1. format:check reports pre-existing issues in src/apktool/ApktoolProvider.ts and src/application/UpdateMaintenance.ts. Those two files are unmodified and fail the same check on upstream/main (6d8f95e3). Lint in that run finished with 623 warnings and 0 errors. Turbo stopped the check script before verify:architecture-guards.
npm run typecheck - exit 0.
npm run knip - exit 0.
npm run verify:architecture-guards - exit 0. Architecture guards passed across 1076 source files.
Pre-push npm run check:fast (typecheck and lint) - exit 0 during git push.

Compatibility, safety, and release

Breaking changes or migration steps: none. The result schema is unchanged. Inspections of pages that contain SVG xlink:href without href gain dom_urls entries. Callers that assumed those links were absent will now see them.
Real Hopper/Ghidra, browser, or OS coverage: headless Google Chrome 148.0.7778.96 on Linux, one loopback page through CdpBrowserProvider.inspectPage. Not the full verify:browser lane. No Hopper, Ghidra, Firefox, or Safari run.
Package or release metadata impact: none. No changelog edit.
Security, privacy, process, or containment review: destinations still go through the existing http(s) allowlist and URL sanitizer. Outside-policy and non-http values stay url: null with an explicit scope. The Chrome test binds 127.0.0.1 and deletes its profile directory. No new process authority.

Review checklist

This PR addresses a concrete problem or an agreed enhancement.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1602 **Author:** [@sathariels](https://github.com/sathariels) **Created:** 10/10/2026 **Status:** ✅ Merged **Merged:** 10/10/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/dom-urls-svg-xlink-href` --- ### 📝 Commits (2) - [`2eb66a1`](https://github.com/morluto/rea/commit/2eb66a15e0cd924064d04f298f8c59e13d390114) fix(browser): report SVG xlink:href links in dom_urls - [`cf3edef`](https://github.com/morluto/rea/commit/cf3edef645fa8eba097a3d721b3244374cf8b75e) fix(browser): cache SVG ancestry during DOM capture ### 📊 Changes **3 files changed** (+566 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/CdpCaptureDocuments.baseUrl.test.ts` (+329 -0) 📝 `src/browser/CdpCaptureDocuments.ts` (+53 -1) ➕ `tests/boundary/browser/cdpSvgXlinkHref.test.ts` (+184 -0) </details> ### 📄 Description  fix(browser): report SVG xlink:href links in dom_urls ## Summary `inspect-web-page` omitted `dom_urls` entries for SVG links written only as `xlink:href`. Chromium still follows those links: an SVG `<a>` with only `xlink:href` has `href.baseVal` set and matches `:any-link`, while an HTML `<a xlink:href>` does not. This reports the SVG value as `href` when `href` itself is missing, so the `dom_urls` schema stays the same. Fixes #1586. ## Problem and expected behavior On an SVG `<a xlink:href="/xl-s2h">` with no `href`, `domMetadata` in `src/browser/CdpCaptureDocuments.ts` looked up only `href`, `src`, `action`, `formaction`, and `poster`. The node was present with `attribute_names` including `xlink:href`, and `dom_urls` had no entry. Expected: the same entry `dom_urls` already produces for `<a href="/xl-s2h">`: attribute `href`, URL resolved against the document base, and the existing origin policy. When both attributes are present, `href` wins, including an empty `href`. An HTML element with only `xlink:href` stays unreported. ## Change and scope `DOMSnapshot.captureSnapshot` does not include a namespace URI (confirmed on Chrome 148: document and node keys have no namespace field). SVG context is therefore structural: - the element nodeName is `svg`, or - an ancestor nodeName is `svg`, and - a closer ancestor is not `foreignObject`. Chrome's HTML parser records those local names as `svg` and `foreignObject`. HTML elements are uppercase (`A`), so an HTML anchor is not SVG context. HTML inside `foreignObject` stops the walk; a nested `svg` starts it again. A cyclic or out-of-range parent index fails closed and does not report `xlink:href`. Inside that context, a missing `href` falls back to the exact attribute name `xlink:href`. The fallback uses the same first-wins attribute map, base URL, and `domDestination` policy as `href`, and is stored with attribute `"href"`. `browserObservationSchemas.ts` is unchanged. Intentionally not included: - MathML and `div` `href` behavior (already reported today; left as-is). - A tag whitelist. `rect` and other SVG elements that carry `xlink:href` are reported the same way REA already reports `href` on elements Chromium does not treat as links. Chrome's `href` IDL reflects `xlink:href` for `a`, `image`, `use`, `script`, `feImage`, `textPath`, gradients, `pattern`, `filter`, and `mpath`, and does not for `rect`. - SVG elements created with `createElementNS` and inserted outside any `svg` ancestor. Chrome does treat an orphan SVG `<a xlink:href>` as a link, but the snapshot cannot distinguish that nodeName `a` from a MathML or XHTML `a`. ## Contract and boundary impact - Semantic owner and earliest changed stage: `domMetadata` in the browser CDP capture adapter, while normalizing a `DOMSnapshot.captureSnapshot` result into `metadata.dom_urls`. No earlier capture stage changes. - CLI and MCP/tool-catalog contract: none. The `dom_urls[].attribute` enum stays `href | src | action | formaction | poster`. Pages that previously omitted SVG `xlink:href` links now include those URLs as `href`. - Provider, bridge, target-format, or platform compatibility: Chrome-family CDP DOM snapshots only. No bridge or target-format change. Behavior depends on Chrome's nodeName casing (`svg` / `foreignObject` / uppercase HTML). - Evidence, artifact, provenance, or reconstruction contract: none. The added entries are the same derived URL records as existing `href` values (resolved URL plus `destination_scope`). Observed attribute names on the DOM node stay `xlink:href`. - Process execution, authorization, cleanup, or containment impact: none. Destination policy and URL sanitization are unchanged. The new test launches a local headless Chrome only when an executable is already installed. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `skill-src/` and never commit build outputs): none. No authored skill, schema, or package metadata change. ## Evidence and regression coverage - Tests added or updated: - `src/browser/CdpCaptureDocuments.baseUrl.test.ts`: xlink-only SVG `<a>` (first duplicate wins), `href` wins in either order, HTML `<a>` and MathML `mrow` with only `xlink:href` stay unlisted, outside-policy origin, SVG `image` and `use` resolved against the document base, `foreignObject` stops SVG context and a nested `svg` resumes it, empty `href` beats `xlink:href`, `javascript:` is unsupported, cyclic and dangling parent indexes are not SVG. - `tests/boundary/browser/cdpSvgXlinkHref.test.ts`: real headless Chrome loads the issue page (plus HTML, image, use, and a foreign origin) and `CdpBrowserProvider.inspectPage` checks `metadata.dom_urls`. Skipped when no Chrome-family executable is installed. - Base reproduction or other evidence: executed. On `6d8f95e3` (upstream/main), before the production change, `npm run test:focused -- src/browser/CdpCaptureDocuments.baseUrl.test.ts` failed 6 tests. Each xlink-only case received `urls: []` instead of an `href` entry. Example: "reports an SVG anchor that only has xlink:href" expected `https://app.example.test/xl-s2h` and received `[]`. The `href`-wins, empty-`href`, and non-SVG cycle cases already passed because no `xlink:href` fallback existed. - User-visible CLI/MCP output (if applicable): `CdpBrowserProvider.inspectPage` is the provider behind CLI and MCP. For the issue anchor the entry is: ```json { "node_index": 3, "attribute": "href", "url": "https://app.example.test/xl-s2h", "destination_scope": "approved" } ``` The real Chrome run resolved `/xl-s2h`, `/href-wins`, `/xl-image`, and `/xl-use` on the loopback origin as `href` / `approved`, reported the `https://cdn.example.test/out` link as `href` / `outside_policy` with `url: null`, and left the HTML `<a xlink:href="/xl-html">` out of `dom_urls`. The compiled CLI was not invoked. - Remaining proof gaps: - An SVG element with no `svg` ancestor (script-inserted orphan) is not reported. - `xlink:href` on SVG elements whose IDL `href` does not read it, such as `rect`, is reported. - Firefox and Safari were not tested. - `npm run verify:browser` was not run. - `npm run docs:check` was not run; no docs or contracts changed. - `npm run check` did not finish `verify:architecture-guards` because `format:check` fails on two pre-existing files. That guard was run separately and passed. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm ci` (Node v24.18.0, npm 11.16.0) - exit 0, 376 packages installed. - `npm run test:focused -- src/browser/CdpCaptureDocuments.baseUrl.test.ts` on the base revision, tests only - exit 1. 6 failed, 26 passed (32). Failing cases received `[]` for xlink-only SVG `a`, HTML-page SVG `a`, outside-policy, image/use, nested svg, and `javascript:` xlink:href. - `npm run test:focused -- src/browser/CdpCaptureDocuments.baseUrl.test.ts` after the fix - exit 0. 32 passed. - `npm run test:focused -- tests/boundary/browser/cdpSvgXlinkHref.test.ts` - exit 0. 1 passed (headless Chrome 148.0.7778.96). Repeated after a small assertion tightening; still 1 passed. - `npm run test:focused -- src/browser/CdpCaptureDocuments.baseUrl.test.ts tests/boundary/browser/cdpSvgXlinkHref.test.ts` - exit 0. 33 passed. - `npm run verify:test-discovery` - exit 0. Discovered all 766 test files exactly once. - `npm run check` - exit 1. `format:check` reports pre-existing issues in `src/apktool/ApktoolProvider.ts` and `src/application/UpdateMaintenance.ts`. Those two files are unmodified and fail the same check on `upstream/main` (`6d8f95e3`). Lint in that run finished with 623 warnings and 0 errors. Turbo stopped the `check` script before `verify:architecture-guards`. - `npm run typecheck` - exit 0. - `npm run knip` - exit 0. - `npm run verify:architecture-guards` - exit 0. Architecture guards passed across 1076 source files. - Pre-push `npm run check:fast` (typecheck and lint) - exit 0 during `git push`. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The result schema is unchanged. Inspections of pages that contain SVG `xlink:href` without `href` gain `dom_urls` entries. Callers that assumed those links were absent will now see them. - Real Hopper/Ghidra, browser, or OS coverage: headless Google Chrome 148.0.7778.96 on Linux, one loopback page through `CdpBrowserProvider.inspectPage`. Not the full `verify:browser` lane. No Hopper, Ghidra, Firefox, or Safari run. - Package or release metadata impact: none. No changelog edit. - Security, privacy, process, or containment review: destinations still go through the existing http(s) allowlist and URL sanitizer. Outside-policy and non-http values stay `url: null` with an explicit scope. The Chrome test binds `127.0.0.1` and deletes its profile directory. No new process authority. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-17 21:12:43 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 01:07:30 +00:00

[PR #2998] [CLOSED] fix(adb): read window focus from the displays section on Android 16 #5550

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#2998

No description provided.

Rows
Columns