morluto/rea

Fork 0

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

[PR #546] [MERGED] fix(browser): ignore duplicate frame navigations and read block-form source map directives #2064

New issue

Closed

opened 2026-10-16 21:13:52 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:13:52 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: cleanup/browser-navigation-and-sourcemaps

📝 Commits (1)

29f94fe fix(browser): ignore duplicate frame navigations and read block-form source map directives

📊 Changes

6 files changed (+149 additions, -4 deletions)

View changed files

📝 src/browser/CdpCaptureEventHandlers.ts (+8 -1)
📝 src/browser/CdpCaptureEventState.ts (+6 -0)
📝 src/browser/CdpCaptureEvents.ts (+10 -1)
📝 src/domain/javascriptStaticAnalysisCalls.ts (+5 -2)
➕ src/domain/javascriptStaticAnalysisSourceMaps.test.ts (+42 -0)
➕ tests/boundary/browser/cdpCaptureNavigation.test.ts (+78 -0)

📄 Description

A-10 from the cleanup sweep. Two real defects, plus two candidates from the review that I checked and did not change, because they were not bugs.

beginFinalCapture cleared navigationDuringCapture = false. That flag is set by handleFrameNavigated for any main-frame navigation, so a page that navigated during the observation window had its flag erased at the moment the final capture began — and the capture reported itself clean while carrying pre-navigation scripts, network and console entries into the inventory of the new document. The flag is now left intact, so CdpPageCapture reports target_changed, matching what the Electron path already did.

Fixed: a duplicate `frameNavigated` aborted a valid capture

Page.frameNavigated re-fires for a document already reported. The handler treated every main-frame event as a navigation, so a repeat with the same URL and the same loaderId failed a good capture with target_changed — a false claim that the target moved. The handler now tracks the committed document identity and ignores re-fires for it, while a genuine URL or loader change is still reported.

Fixed: block-form source map directives were lost

Minifiers commonly emit /*# sourceMappingURL=app.js.map */. The matcher only handled the // form, and for the block form it produced no match at all — so a real footer yielded no mapping. (The review predicted it would capture the trailing */; the actual failure was worse — nothing at all.) Both forms are now read, with the terminator excluded.

Checked and left alone

processComparison.classifyCollection inventing added/removed from emptiness. Not a bug: compareProcessCapture returns truncatedComparison() and never reaches compareDimensions when either capture is truncated. Emptiness cannot become a false added.
sanitizeEndpointCandidate / sanitizeSource stripping ?/# from bare paths in their catch fallback. Not reachable: new URL(value, "https://rea.invalid") succeeds for any relative value, so the fallback only runs on input that is not a path. Verified src/query?test.ts and page#1.js keep their characters.

Tests: new cdpCaptureNavigation.test.ts covering the re-fire, the genuine change, the observation-window navigation and a settled window, plus an end-to-end inspectPage against the fake CDP browser; new source-map directive suite covering all four forms. Full suite green: 1958 passed, typecheck and lint clean (0 errors).

_{🔄 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/546 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/5/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `cleanup/browser-navigation-and-sourcemaps` --- ### 📝 Commits (1) - [`29f94fe`](https://github.com/morluto/rea/commit/29f94fe5675866bde42d6b998973a041cb72a4ed) fix(browser): ignore duplicate frame navigations and read block-form source map directives ### 📊 Changes **6 files changed** (+149 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/CdpCaptureEventHandlers.ts` (+8 -1) 📝 `src/browser/CdpCaptureEventState.ts` (+6 -0) 📝 `src/browser/CdpCaptureEvents.ts` (+10 -1) 📝 `src/domain/javascriptStaticAnalysisCalls.ts` (+5 -2) ➕ `src/domain/javascriptStaticAnalysisSourceMaps.test.ts` (+42 -0) ➕ `tests/boundary/browser/cdpCaptureNavigation.test.ts` (+78 -0) </details> ### 📄 Description A-10 from the cleanup sweep. Two real defects, plus two candidates from the review that I checked and **did not** change, because they were not bugs. ## Fixed: a navigation during the observation window was silently forgotten `beginFinalCapture` cleared `navigationDuringCapture = false`. That flag is set by `handleFrameNavigated` for any main-frame navigation, so a page that navigated *during the observation window* had its flag erased at the moment the final capture began — and the capture reported itself clean while carrying pre-navigation scripts, network and console entries into the inventory of the *new* document. The flag is now left intact, so `CdpPageCapture` reports `target_changed`, matching what the Electron path already did. ## Fixed: a duplicate `frameNavigated` aborted a valid capture `Page.frameNavigated` re-fires for a document already reported. The handler treated every main-frame event as a navigation, so a repeat with the same URL *and* the same `loaderId` failed a good capture with `target_changed` — a false claim that the target moved. The handler now tracks the committed document identity and ignores re-fires for it, while a genuine URL or loader change is still reported. ## Fixed: block-form source map directives were lost Minifiers commonly emit `/*# sourceMappingURL=app.js.map */`. The matcher only handled the `//` form, and for the block form it produced **no match at all** — so a real footer yielded no mapping. (The review predicted it would capture the trailing `*/`; the actual failure was worse — nothing at all.) Both forms are now read, with the terminator excluded. ## Checked and left alone - **`processComparison.classifyCollection` inventing `added`/`removed` from emptiness.** Not a bug: `compareProcessCapture` returns `truncatedComparison()` and never reaches `compareDimensions` when either capture is truncated. Emptiness cannot become a false `added`. - **`sanitizeEndpointCandidate` / `sanitizeSource` stripping `?`/`#` from bare paths in their `catch` fallback.** Not reachable: `new URL(value, "https://rea.invalid")` succeeds for any relative value, so the fallback only runs on input that is not a path. Verified `src/query?test.ts` and `page#1.js` keep their characters. Tests: new `cdpCaptureNavigation.test.ts` covering the re-fire, the genuine change, the observation-window navigation and a settled window, plus an end-to-end `inspectPage` against the fake CDP browser; new source-map directive suite covering all four forms. Full suite green: 1958 passed, typecheck and lint clean (0 errors). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:13:52 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 00:57:51 +00:00

[PR #2066] [CLOSED] test(adb): restore fixtures and the quoting regression after listing changes #5000

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#2064

No description provided.

Rows
Columns