From d673ce14be771eeb63a5c4f826719fb93794f2db Mon Sep 17 00:00:00 2001 From: Snider Date: Wed, 15 Apr 2026 14:23:44 +0100 Subject: [PATCH] fix(webview): use protocol-correct history navigation Replace the invalid Page.goBackOrForward call with Page.getNavigationHistory and Page.navigateToHistoryEntry, and update the regression coverage plus local docs. Co-Authored-By: Virgil --- audit_issue2_test.go | 40 +++++++++++++++++++++++++++++----------- docs/history.md | 2 +- specs/RFC.md | 4 ++-- webview.go | 34 ++++++++++++++++++++++++++++++++-- 4 files changed, 64 insertions(+), 16 deletions(-) diff --git a/audit_issue2_test.go b/audit_issue2_test.go index bd11a35..e278b2c 100644 --- a/audit_issue2_test.go +++ b/audit_issue2_test.go @@ -672,7 +672,7 @@ func TestExceptionWatcherWaitForException_Good_PreservesExistingHandlers(t *test } } -func TestWebviewGoBack_Good_UsesGoBackOrForwardAndWaitsForLoad(t *testing.T) { +func TestWebviewGoBack_Good_UsesNavigationHistoryAndWaitsForLoad(t *testing.T) { server := newFakeCDPServer(t) target := server.primaryTarget() @@ -681,9 +681,18 @@ func TestWebviewGoBack_Good_UsesGoBackOrForwardAndWaitsForLoad(t *testing.T) { methods = append(methods, msg.Method) switch msg.Method { - case "Page.goBackOrForward": - if got, ok := msg.Params["delta"].(float64); !ok || got != -1 { - t.Fatalf("goBackOrForward delta = %v, want -1", msg.Params["delta"]) + case "Page.getNavigationHistory": + target.reply(msg.ID, map[string]any{ + "currentIndex": float64(1), + "entries": []any{ + map[string]any{"id": float64(11)}, + map[string]any{"id": float64(12)}, + map[string]any{"id": float64(13)}, + }, + }) + case "Page.navigateToHistoryEntry": + if got, ok := msg.Params["entryId"].(float64); !ok || got != 11 { + t.Fatalf("navigateToHistoryEntry entryId = %v, want 11", msg.Params["entryId"]) } target.reply(msg.ID, map[string]any{}) case "Runtime.evaluate": @@ -709,22 +718,31 @@ func TestWebviewGoBack_Good_UsesGoBackOrForwardAndWaitsForLoad(t *testing.T) { t.Fatalf("GoBack returned error: %v", err) } - if len(methods) != 2 { - t.Fatalf("expected 2 CDP calls, got %d (%v)", len(methods), methods) + if len(methods) != 3 { + t.Fatalf("expected 3 CDP calls, got %d (%v)", len(methods), methods) } - if methods[0] != "Page.goBackOrForward" || methods[1] != "Runtime.evaluate" { + if methods[0] != "Page.getNavigationHistory" || methods[1] != "Page.navigateToHistoryEntry" || methods[2] != "Runtime.evaluate" { t.Fatalf("unexpected call sequence: %v", methods) } } -func TestWebviewGoForward_Good_UsesGoBackOrForwardAndWaitsForLoad(t *testing.T) { +func TestWebviewGoForward_Good_UsesNavigationHistoryAndWaitsForLoad(t *testing.T) { server := newFakeCDPServer(t) target := server.primaryTarget() target.onMessage = func(target *fakeCDPTarget, msg cdpMessage) { switch msg.Method { - case "Page.goBackOrForward": - if got, ok := msg.Params["delta"].(float64); !ok || got != 1 { - t.Fatalf("goBackOrForward delta = %v, want 1", msg.Params["delta"]) + case "Page.getNavigationHistory": + target.reply(msg.ID, map[string]any{ + "currentIndex": float64(1), + "entries": []any{ + map[string]any{"id": float64(11)}, + map[string]any{"id": float64(12)}, + map[string]any{"id": float64(13)}, + }, + }) + case "Page.navigateToHistoryEntry": + if got, ok := msg.Params["entryId"].(float64); !ok || got != 13 { + t.Fatalf("navigateToHistoryEntry entryId = %v, want 13", msg.Params["entryId"]) } target.reply(msg.ID, map[string]any{}) case "Runtime.evaluate": diff --git a/docs/history.md b/docs/history.md index 54aebc6..6c6da21 100644 --- a/docs/history.md +++ b/docs/history.md @@ -73,7 +73,7 @@ The JavaScript Promise used in `waitForZoneStability` has an internal 5-second ` ### CloseTab Implementation -`CDPClient.CloseTab` calls `Browser.close`, which closes the entire browser rather than just the tab. The correct CDP command for closing a single tab is `Target.closeTarget` with the target's ID extracted from the WebSocket URL. This is a bug. +`CDPClient.CloseTab` now uses `Target.closeTarget` with the current target ID extracted from the WebSocket URL, so closing one tab no longer tears down the entire browser. --- diff --git a/specs/RFC.md b/specs/RFC.md index bdf4b45..96be2e1 100644 --- a/specs/RFC.md +++ b/specs/RFC.md @@ -423,8 +423,8 @@ Methods: - `GetHTML(selector string) (string, error)`: Returns `document.documentElement.outerHTML` when `selector` is empty; otherwise returns `document.querySelector(selector)?.outerHTML || ""`. - `GetTitle() (string, error)`: Evaluates `document.title` and requires the result to be a string. - `GetURL() (string, error)`: Evaluates `window.location.href` and requires the result to be a string. -- `GoBack() error`: Calls `Page.goBackOrForward` with `delta: -1`. -- `GoForward() error`: Calls `Page.goBackOrForward` with `delta: 1`. +- `GoBack() error`: Calls `Page.getNavigationHistory`, selects the previous entry, and then calls `Page.navigateToHistoryEntry`. +- `GoForward() error`: Calls `Page.getNavigationHistory`, selects the next entry, and then calls `Page.navigateToHistoryEntry`. - `Navigate(url string) error`: Calls `Page.navigate` and then polls `document.readyState` every 100 ms until it becomes `"complete"` or the timeout expires. - `QuerySelector(selector string) (*ElementInfo, error)`: Fetches the document root, runs `DOM.querySelector`, errors when the selector does not resolve, and returns `ElementInfo` for the matched node. - `QuerySelectorAll(selector string) ([]*ElementInfo, error)`: Runs `DOM.querySelectorAll` and returns one `ElementInfo` per node ID whose metadata lookup succeeds. Nodes whose metadata fetch fails are skipped. diff --git a/webview.go b/webview.go index 299f635..4e50c50 100644 --- a/webview.go +++ b/webview.go @@ -415,8 +415,38 @@ func (wv *Webview) navigateHistory(delta int, scope string) error { ctx, cancel := context.WithTimeout(wv.ctx, wv.timeout) defer cancel() - _, err := wv.client.Call(ctx, "Page.goBackOrForward", map[string]any{ - "delta": delta, + history, err := wv.client.Call(ctx, "Page.getNavigationHistory", nil) + if err != nil { + return coreerr.E(scope, "failed to get navigation history", err) + } + + currentIndexFloat, ok := history["currentIndex"].(float64) + if !ok { + return coreerr.E(scope, "invalid navigation history index", nil) + } + + entries, ok := history["entries"].([]any) + if !ok { + return coreerr.E(scope, "invalid navigation history entries", nil) + } + + targetIndex := int(currentIndexFloat) + delta + if targetIndex < 0 || targetIndex >= len(entries) { + return coreerr.E(scope, "no navigation history entry available", nil) + } + + entry, ok := entries[targetIndex].(map[string]any) + if !ok { + return coreerr.E(scope, "invalid navigation history entry", nil) + } + + entryIDFloat, ok := entry["id"].(float64) + if !ok { + return coreerr.E(scope, "invalid navigation history entry id", nil) + } + + _, err = wv.client.Call(ctx, "Page.navigateToHistoryEntry", map[string]any{ + "entryId": int(entryIDFloat), }) if err != nil { return coreerr.E(scope, "failed to navigate history", err)