From bf6e18b29086962d6be97ed494720e8d21bd3b21 Mon Sep 17 00:00:00 2001 From: omartelo Date: Fri, 30 Oct 2026 11:30:29 -0300 Subject: [PATCH] fix(browser): open the file chooser for a file input, and land a Leave on a busy machine A headless Chrome has no file chooser, so a click on a file input in the session browser did nothing. The page now intercepts the chooser and opens the system one the attach button uses, for one file or several as the input asks, while a pane shows the page. A beforeunload accepted too soon has Chrome abort the navigation it held and reload the page, and how soon is too soon grows with load (about 450 ms idle, 2 to 3 s with Chromium held to 30% of a core), which made TestChromeADialogWaitsForTheAgentsAnswer fail 6 in 10 under the gate's load. A leave now watches for that reload and takes the held navigation up again once it lands. --- CHANGELOG.md | 5 + docs/browser-tab.md | 12 +- docs/ceilings.md | 13 ++- internal/browser/browser.go | 4 + internal/browser/chrome.go | 8 +- internal/browser/chrome_dialog.go | 8 +- internal/browser/chrome_files.go | 44 ++++++++ internal/browser/chrome_leave.go | 154 ++++++++++++++++++++++++++ internal/browser/chrome_leave_test.go | 75 +++++++++++++ internal/browser/chrome_tabs.go | 4 +- internal/browser/chrome_view.go | 26 +++++ internal/browser/fake_test.go | 3 + internal/browser/files.go | 40 +++++++ internal/browser/files_live_test.go | 70 ++++++++++++ internal/browser/files_test.go | 47 ++++++++ internal/browser/page.go | 5 + internal/project/picker.go | 8 ++ main.go | 1 + 18 files changed, 521 insertions(+), 6 deletions(-) create mode 100644 internal/browser/chrome_files.go create mode 100644 internal/browser/chrome_leave.go create mode 100644 internal/browser/chrome_leave_test.go create mode 100644 internal/browser/files.go create mode 100644 internal/browser/files_live_test.go create mode 100644 internal/browser/files_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 9961ff9bb..eb18db4f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The Pulls screen opens on an index of the project's pull requests across the whole screen, and a pull request opens in its place instead of beside the list. A back button over its title returns to the index as you left it; beside it, the number opens a menu of the other pull requests, and previous and next walk the index's filter. On the Files tab only the file tree sits beside the diff, which gains the width the list took. The Overview puts the description beside a column with the reviewers, checks, mergeability and the checkout the branch sits in; the tab strip ends with the pull request's size. Open in Session and Merge stay in the header, and the hand-off, Approve, Refresh and Open on GitHub move to the ⋯ menu. On the index, j and k or the arrows move, Enter opens and / goes to the filter; on a pull request, Escape goes back and Alt with an arrow steps to the previous or next one. +### Fixed + +- A file input in the session browser, such as a profile picture upload, opens the system's file chooser when you click it, and takes several files when the input allows it. Before, the click did nothing. +- Leave on a page in the session browser that asks before you go no longer reloads the page instead on a busy machine; the browser goes where it was going. + ## [0.83.0] - 2026-10-29 ### Added diff --git a/docs/browser-tab.md b/docs/browser-tab.md index 35f4629a2..b4c0fb80e 100644 --- a/docs/browser-tab.md +++ b/docs/browser-tab.md @@ -98,7 +98,17 @@ input. lich answers no dialog on its own: Accepting a `beforeunload` less than a second after it opened waits out the rest of that second: Chrome aborts the navigation it held when it is accepted -sooner, and reloads the page, which asks again. +sooner, and reloads the page, which asks again. On a busy machine that second +can still be too soon; lich then takes the leave up again once the reload +lands, so Leave goes where it was going. + +## File inputs + +A click on a file input opens the system's file chooser, the one the footer's +attach button opens, for one file or several as the input asks; what the +person picks fills the input, and Cancel leaves it as it was. It opens only +while a pane shows the page: an agent's click on a file input in a page nobody +is looking at leaves it empty, and no tool fills one. ## Languages of the page answer diff --git a/docs/ceilings.md b/docs/ceilings.md index d44baf8e6..578ba653e 100644 --- a/docs/ceilings.md +++ b/docs/ceilings.md @@ -1065,7 +1065,11 @@ work when nobody knows it and that the call site never shows. The mechanism and stood, not as the dialog's answer left it. A dialog the page opens on a timer interrupts whichever call is running. Accepting a `beforeunload` less than a second after it opened is held to that second: accepted sooner, Chrome aborts the navigation it held and reloads the page, which asks again (3 of 3 at 300 ms, 3 of 6 at 400 ms, - none of 30 from 500 ms; Chrome for Testing 155 and Helium 0.19.2.1). The agent hears of a dialog the person + none of 30 from 500 ms; Chrome for Testing 155 and Helium 0.19.2.1). That bound grows with the machine's load + (2 to 3 s with Chromium held to 30% of a core), so a reload after an accepted `beforeunload` is taken for an + abort: lich accepts the reload's own `beforeunload` unseen and loads the held URL again once the reload lands, + up to three times, and the agent's answer waits up to 15 seconds for it. A held navigation that a redirect + would have sent elsewhere is taken up at the URL it started with. The agent hears of a dialog the person answered, at most twenty between two page answers, but not of one the agent itself answered. A held page cannot lay out, so a pane opened or resized meanwhile shows it at the size it had, and lays it out at the pane's once the dialog closes; a screenshot fails until then. On Chrome 153 and older, which answers a screencast's start and @@ -1073,6 +1077,13 @@ work when nobody knows it and that the call site never shows. The mechanism and (Chrome 151 on the macos-15-intel runner, measured; 154 on answers at once). A window opened with Open in a window was not tried with a dialog. +- **A file input in a session's page opens the OS file chooser only while a pane shows the page** + (`internal/browser/files.go`, `chrome_files.go`): headless Chrome has no chooser of its own, so lich intercepts + it and opens the one the attach button uses, one file or several as the input asks. An agent's click on a file + input in a page nobody is looking at leaves the input empty, and no tool fills one; an agent's click while a + pane shows the page opens the chooser in front of the person. The input's `accept` filter is not passed on: the + chooser lists every file and the page refuses a type it does not take. + ## Languages - **The prompt language reaches a running session only through lich itself** (`internal/prompt`): text lich diff --git a/internal/browser/browser.go b/internal/browser/browser.go index 615c27ea8..1c45913d7 100644 --- a/internal/browser/browser.go +++ b/internal/browser/browser.go @@ -70,6 +70,9 @@ type Service struct { // isLocal and local are SetLocal's. isLocal func(raw string) bool local func(owner, raw string) (string, error) + // pickFile and pickFiles are SetPicker's. + pickFile func(title string) (string, error) + pickFiles func(title string) ([]string, error) } type session struct { @@ -286,6 +289,7 @@ func (s *Service) start(spec launchSpec) (Handle, error) { id: newID(), owner: spec.owner, dir: spec.dir, url: spec.url, headed: spec.headed, byAgent: spec.byAgent, page: page, lastUsed: s.now(), } + page.OnFileChooser(s.fileSink(sess)) page.Listen(s.frameSink(sess), s.infoSink(sess), s.downloadSink(sess), s.consoleSink(sess), s.dialogSink(sess), s.tabsSink(sess)) view := s.unwatchedView() if err := runPage(func(ctx context.Context) error { diff --git a/internal/browser/chrome.go b/internal/browser/chrome.go index 7f901d674..bd9c9193b 100644 --- a/internal/browser/chrome.go +++ b/internal/browser/chrome.go @@ -60,7 +60,7 @@ func chromeLaunch(dir string, headed bool) (Page, error) { } allocCtx, allocCancel := chromedp.NewExecAllocator(context.Background(), opts...) ctx, cancel := chromedp.NewContext(allocCtx) - if err := chromedp.Run(ctx); err != nil { + if err := chromedp.Run(ctx, interceptFiles); err != nil { cancel() allocCancel() return nil, err @@ -107,6 +107,11 @@ type chromePage struct { castMu sync.Mutex castLast chan struct{} casting bool + // leaveMu guards held, the navigation the open beforeunload holds, and + // leave, the one an accepted beforeunload left for (chrome_leave.go). + leaveMu sync.Mutex + held *leave + leave *leave } // downloadFolder is downloadDir, which the live tests point at a folder of @@ -234,6 +239,7 @@ var errNavAborted = errors.New("page load error " + abortedText) const navGrace = 150 * time.Millisecond func (p *chromePage) Settle(ctx context.Context) error { + p.awaitLeave(ctx) return p.run(ctx, settleAfter(func(ctx context.Context) error { t := time.NewTimer(navGrace) defer t.Stop() diff --git a/internal/browser/chrome_dialog.go b/internal/browser/chrome_dialog.go index 172694250..013d128ca 100644 --- a/internal/browser/chrome_dialog.go +++ b/internal/browser/chrome_dialog.go @@ -82,5 +82,11 @@ func (p *chromePage) layOut(v Viewport) { func (p *chromePage) AnswerDialog(ctx context.Context, accept bool, text string) error { tabCtx, cancel := withPageDeadline(p.tabs.dialogHolder().ctx, ctx) defer cancel() - return chromedp.Run(tabCtx, page.HandleJavaScriptDialog(accept).WithPromptText(text)) + // The leave is in place before the page can land on its new document. + l := p.leaving(accept) + if err := chromedp.Run(tabCtx, page.HandleJavaScriptDialog(accept).WithPromptText(text)); err != nil { + p.forgetLeave(l) + return err + } + return nil } diff --git a/internal/browser/chrome_files.go b/internal/browser/chrome_files.go new file mode 100644 index 000000000..07cef877c --- /dev/null +++ b/internal/browser/chrome_files.go @@ -0,0 +1,44 @@ +package browser + +import ( + "context" + "log/slog" + + "github.com/chromedp/cdproto/dom" + "github.com/chromedp/cdproto/page" + "github.com/chromedp/chromedp" +) + +// interceptFiles turns a file input's chooser into an event: a headless +// Chrome has no dialog to show, and a click on the input did nothing. +var interceptFiles = page.SetInterceptFileChooserDialog(true) + +func (p *chromePage) OnFileChooser(pick func(page.FileChooserOpenedMode) ([]string, error)) { + p.listen.mu.Lock() + p.listen.onFileChooser = pick + p.listen.mu.Unlock() +} + +// chooseFiles fills the file input of t that asked for files with what the +// picker answers. +func (p *chromePage) chooseFiles(t *tab, e *page.EventFileChooserOpened) { + p.listen.mu.Lock() + pick := p.listen.onFileChooser + p.listen.mu.Unlock() + if pick == nil { + return + } + paths, err := pick(e.Mode) + if err != nil { + slog.Warn("browser: open the file chooser", "err", err) + return + } + if len(paths) == 0 { + return + } + ctx, cancel := context.WithTimeout(t.ctx, pageWait) + defer cancel() + if err := chromedp.Run(ctx, dom.SetFileInputFiles(paths).WithBackendNodeID(e.BackendNodeID)); err != nil && p.Alive() { + slog.Warn("browser: fill a file input", "err", err) + } +} diff --git a/internal/browser/chrome_leave.go b/internal/browser/chrome_leave.go new file mode 100644 index 000000000..1dd2e0a7c --- /dev/null +++ b/internal/browser/chrome_leave.go @@ -0,0 +1,154 @@ +package browser + +import ( + "context" + "log/slog" + "time" + + "github.com/chromedp/cdproto/page" + "github.com/chromedp/chromedp" +) + +// A beforeunload accepted too soon after it opened has Chrome abort the +// navigation it held and reload the page instead, and how soon is too soon +// grows with the machine's load: about 450 ms on an idle one, 2 to 3 s with +// the browser held to 30% of a core (Chromium 155, measured 2026-10-30). No +// CDP event marks the moment, so beforeunloadSettle is a guess, and a leave +// watches for the reload and goes where the person said to go once it lands. + +// leaveTries is how many aborted leaves are taken up again before the page +// is left where Chrome put it. +const leaveTries = 3 + +// leaveWait bounds how long Settle waits on a leave: a held navigation that +// turned into a download never lands. +const leaveWait = 15 * time.Second + +// leave is a beforeunload lich accepted, until the navigation it held lands. +type leave struct { + tab *tab + url string + // aborted: Chrome reloaded the page instead. + aborted bool + tries int + done chan struct{} +} + +// holdLeave keeps url, the navigation the dialog t just opened holds, for +// the accept that would leave for it; "" for a dialog that holds none. +func (p *chromePage) holdLeave(t *tab, url string) { + p.leaveMu.Lock() + defer p.leaveMu.Unlock() + p.held = &leave{tab: t, url: url} +} + +// leaving is the answer to the open dialog: the leave an accepted +// beforeunload starts, nil for any other answer. +func (p *chromePage) leaving(accept bool) *leave { + p.leaveMu.Lock() + defer p.leaveMu.Unlock() + held := p.held + p.held = nil + if !accept || held == nil || held.url == "" { + return nil + } + held.done = make(chan struct{}) + p.leave = held + return held +} + +// forgetLeave drops l, the leave of an answer that never reached Chrome. +func (p *chromePage) forgetLeave(l *leave) { + p.leaveMu.Lock() + defer p.leaveMu.Unlock() + if l != nil && p.leave == l { + close(l.done) + p.leave = nil + } +} + +// leaveStarted marks the leave on t aborted when Chrome starts reloading the +// page instead. +func (p *chromePage) leaveStarted(t *tab, kind page.FrameStartedNavigatingNavigationType) { + p.leaveMu.Lock() + defer p.leaveMu.Unlock() + if l := p.leave; l != nil && l.tab == t && kind == page.FrameStartedNavigatingNavigationTypeReload { + l.aborted = true + } +} + +// leaveLanded ends the leave on t once a document lands, or, when it is the +// reload of an aborted one, goes for the held navigation again. The reloaded +// document has no user activation, so that navigation asks nothing. +func (p *chromePage) leaveLanded(t *tab) { + p.leaveMu.Lock() + defer p.leaveMu.Unlock() + l := p.leave + if l == nil || l.tab != t { + return + } + if !l.aborted || l.tries >= leaveTries { + close(l.done) + p.leave = nil + return + } + l.aborted = false + l.tries++ + go p.leaveAgain(t, l.url) +} + +func (p *chromePage) leaveAgain(t *tab, url string) { + ctx, cancel := context.WithTimeout(t.ctx, pageWait) + defer cancel() + if err := chromedp.Run(ctx, chromedp.ActionFunc(func(ctx context.Context) error { + _, _, _, _, err := page.Navigate(url).Do(ctx) + return err + })); err != nil && p.Alive() { + slog.Warn("browser: take up a leave Chrome aborted", "url", url, "err", err) + } +} + +// reloadAsks accepts, without showing it, the beforeunload a reload of an +// aborted leave on t opens, since the person already chose to leave; false +// when there is no such leave. It waits longer each try for the same reason +// the first accept waited. +func (p *chromePage) reloadAsks(t *tab) bool { + p.leaveMu.Lock() + l := p.leave + ok := l != nil && l.tab == t && l.aborted + var wait time.Duration + if ok { + wait = time.Duration(l.tries+2) * beforeunloadSettle + } + p.leaveMu.Unlock() + if !ok { + return false + } + go func() { + time.Sleep(wait) + ctx, cancel := context.WithTimeout(t.ctx, pageWait) + defer cancel() + if err := chromedp.Run(ctx, page.HandleJavaScriptDialog(true)); err != nil && p.Alive() { + slog.Warn("browser: accept the beforeunload of a reload", "err", err) + } + }() + return true +} + +// awaitLeave waits for a leave in flight to land, bounded by ctx and +// leaveWait. +func (p *chromePage) awaitLeave(ctx context.Context) { + p.leaveMu.Lock() + l := p.leave + p.leaveMu.Unlock() + if l == nil { + return + } + t := time.NewTimer(leaveWait) + defer t.Stop() + select { + case <-l.done: + case <-ctx.Done(): + case <-t.C: + } +} diff --git a/internal/browser/chrome_leave_test.go b/internal/browser/chrome_leave_test.go new file mode 100644 index 000000000..8936cfbb5 --- /dev/null +++ b/internal/browser/chrome_leave_test.go @@ -0,0 +1,75 @@ +package browser + +import ( + "context" + "testing" + + "github.com/chromedp/cdproto/page" +) + +func landedPage(t *testing.T) (*chromePage, *tab) { + t.Helper() + ctx, cancel := context.WithCancel(context.Background()) + // A gone page: the re-navigation of an aborted leave fails quietly. + cancel() + return &chromePage{ctx: ctx}, &tab{ctx: ctx, id: "t"} +} + +func isDone(l *leave) bool { + select { + case <-l.done: + return true + default: + return false + } +} + +func TestALeaveEndsWhereTheNavigationLands(t *testing.T) { + p, tb := landedPage(t) + p.holdLeave(tb, "http://x/next") + l := p.leaving(true) + if l == nil { + t.Fatal("an accepted beforeunload left for nothing") + } + p.leaveStarted(tb, page.FrameStartedNavigatingNavigationTypeDifferentDocument) + p.leaveLanded(tb) + if !isDone(l) || p.leave != nil { + t.Fatal("the leave outlived the document it landed on") + } +} + +func TestAnAbortedLeaveIsTakenUpAgainUntilItsTriesRunOut(t *testing.T) { + p, tb := landedPage(t) + p.holdLeave(tb, "http://x/next") + l := p.leaving(true) + for try := 1; try <= leaveTries; try++ { + p.leaveStarted(tb, page.FrameStartedNavigatingNavigationTypeReload) + if !p.reloadAsks(tb) { + t.Fatalf("try %d: the reload's beforeunload was left to show", try) + } + p.leaveLanded(tb) + if isDone(l) || l.tries != try { + t.Fatalf("try %d: done %v after %d tries, want it taken up again", try, isDone(l), l.tries) + } + } + p.leaveStarted(tb, page.FrameStartedNavigatingNavigationTypeReload) + p.leaveLanded(tb) + if !isDone(l) { + t.Fatal("the leave went on past its tries") + } +} + +func TestOnlyAnAcceptedBeforeunloadLeaves(t *testing.T) { + p, tb := landedPage(t) + p.holdLeave(tb, "http://x/next") + if p.leaving(false) != nil { + t.Fatal("a dismissed beforeunload left") + } + p.holdLeave(tb, "") + if p.leaving(true) != nil { + t.Fatal("an accepted alert left") + } + if p.reloadAsks(tb) { + t.Fatal("a beforeunload with no leave was accepted unseen") + } +} diff --git a/internal/browser/chrome_tabs.go b/internal/browser/chrome_tabs.go index b86a0f82c..47a3b78fd 100644 --- a/internal/browser/chrome_tabs.go +++ b/internal/browser/chrome_tabs.go @@ -66,7 +66,7 @@ func (p *chromePage) bringIn(id target.ID, url string, opener *tab) { ctx, cancel := chromedp.NewContext(p.ctx, chromedp.WithTargetID(id)) t := &tab{ctx: ctx, cancel: cancel, id: id, opener: opener, url: url} p.listenTab(t) - if err := chromedp.Run(ctx); err != nil { + if err := chromedp.Run(ctx, interceptFiles); err != nil { cancel() if p.Alive() { slog.Warn("browser: attach to a tab the page opened", "url", url, "err", err) @@ -88,7 +88,7 @@ func (p *chromePage) NewTab(context.Context) error { ctx, cancel := chromedp.NewContext(p.ctx) t := &tab{ctx: ctx, cancel: cancel, url: "about:blank"} p.listenTab(t) - if err := chromedp.Run(ctx); err != nil { + if err := chromedp.Run(ctx, interceptFiles); err != nil { cancel() return err } diff --git a/internal/browser/chrome_view.go b/internal/browser/chrome_view.go index c31b1d2c3..b5bf0fae6 100644 --- a/internal/browser/chrome_view.go +++ b/internal/browser/chrome_view.go @@ -31,6 +31,7 @@ type listeners struct { onConsoleError func(text string) onDialog func(*api.BrowserDialog) onTabs func([]api.BrowserTab) + onFileChooser func(page.FileChooserOpenedMode) ([]string, error) } func (p *chromePage) Listen(onFrame func(data string), onInfo func(url, title string), onDownload func(guid string, d api.BrowserDownload), onConsoleError func(text string), onDialog func(*api.BrowserDialog), onTabs func([]api.BrowserTab)) { @@ -46,9 +47,29 @@ func (p *chromePage) Listen(onFrame func(data string), onInfo func(url, title st // listenTab hands on the events of t while it is the front tab, and its // dialogs from behind too. func (p *chromePage) listenTab(t *tab) { + // started is the URL of the tab's last navigation, the one a beforeunload + // that opens next holds. + var started string chromedp.ListenTarget(t.ctx, func(ev any) { switch e := ev.(type) { + case *page.EventFrameStartedNavigating: + if string(e.FrameID) == string(t.id) { + started = e.URL + p.leaveStarted(t, e.NavigationType) + } + case *page.EventFrameNavigated: + if e.Frame.ParentID == "" { + p.leaveLanded(t) + } case *page.EventJavascriptDialogOpening: + held := "" + if e.Type == page.DialogTypeBeforeunload { + if p.reloadAsks(t) { + return + } + held = started + } + p.holdLeave(t, held) p.tabs.setDialog(t) // The service holds the dialog before a call the gate releases // asks it which one. @@ -64,6 +85,11 @@ func (p *chromePage) listenTab(t *tab) { } p.dialogShown(nil) return + case *page.EventFileChooserOpened: + // The person picks for as long as they like, and every event of + // the tab waits behind this listener. + go p.chooseFiles(t, e) + return } if p.tabs.frontTab() == t { p.frontEvent(ev) diff --git a/internal/browser/fake_test.go b/internal/browser/fake_test.go index 58b70c55f..59f7380bd 100644 --- a/internal/browser/fake_test.go +++ b/internal/browser/fake_test.go @@ -7,6 +7,7 @@ import ( "sync" "github.com/chromedp/cdproto/network" + "github.com/chromedp/cdproto/page" "github.com/lichdotdev/lich/api" ) @@ -242,6 +243,8 @@ func (p *fakePage) Listen(onFrame func(string), onInfo func(url, title string), p.onTabs = onTabs } +func (p *fakePage) OnFileChooser(func(page.FileChooserOpenedMode) ([]string, error)) {} + // NewTab, SelectTab and CloseTab record the tab action and fail with // tabErr. func (p *fakePage) NewTab(context.Context) error { return p.tabAction("new", "") } diff --git a/internal/browser/files.go b/internal/browser/files.go new file mode 100644 index 000000000..1fa46ab5f --- /dev/null +++ b/internal/browser/files.go @@ -0,0 +1,40 @@ +package browser + +import ( + "github.com/chromedp/cdproto/page" +) + +// chooseFileTitle and chooseFilesTitle are the file chooser's own titles, +// which are all the dialog says about why it opened, as drop.attachTitle. +const ( + chooseFileTitle = "Choose File" + chooseFilesTitle = "Choose Files" +) + +// SetPicker wires the host's file picker (internal/project), which a page's +// file input opens. Startup wiring, called before anything serves. +func (s *Service) SetPicker(pickFile func(title string) (string, error), pickFiles func(title string) ([]string, error)) { + s.pickFile, s.pickFiles = pickFile, pickFiles +} + +// fileSink opens the picker for a file input of sess's page, in the input's +// mode. Only for a page someone sees: a chooser the agent opens on a page no +// pane shows would pop up a dialog nobody asked for, so the input stays empty. +func (s *Service) fileSink(sess *session) func(page.FileChooserOpenedMode) ([]string, error) { + return func(mode page.FileChooserOpenedMode) ([]string, error) { + s.mu.Lock() + seen := sess.watching || sess.headed + s.mu.Unlock() + if !seen || s.pickFile == nil { + return nil, nil + } + if mode == page.FileChooserOpenedModeSelectMultiple { + return s.pickFiles(chooseFilesTitle) + } + path, err := s.pickFile(chooseFileTitle) + if path == "" { + return nil, err + } + return []string{path}, err + } +} diff --git a/internal/browser/files_live_test.go b/internal/browser/files_live_test.go new file mode 100644 index 000000000..e9ec97364 --- /dev/null +++ b/internal/browser/files_live_test.go @@ -0,0 +1,70 @@ +package browser + +import ( + "fmt" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/lichdotdev/lich/api" + "github.com/lichdotdev/lich/internal/chromium" +) + +// filesPage titles the page with what each input holds once it changes. +const filesPage = `Files + +` + +func TestChromeAFileInputFillsWithWhatThePersonPicks(t *testing.T) { + if _, err := chromium.FindBrowser(exec.LookPath); err != nil { + t.Skip("no Chromium-family browser on this machine") + } + testDownloads(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/html") + fmt.Fprint(w, filesPage) + })) + t.Cleanup(srv.Close) + dir := t.TempDir() + a, b := filepath.Join(dir, "a.png"), filepath.Join(dir, "b.png") + for _, f := range []string{a, b} { + if err := os.WriteFile(f, []byte("png"), 0o600); err != nil { + t.Fatal(err) + } + } + s := newService(chromeLaunch, func(string, string) (string, error) { return t.TempDir(), nil }, time.Now, idleAfter) + t.Cleanup(func() { + if err := s.Cleanup(); err != nil { + t.Error(err) + } + }) + s.SetPicker(func(string) (string, error) { return a, nil }, func(string) ([]string, error) { return []string{a, b}, nil }) + if _, err := s.Open("s", srv.URL); err != nil { + t.Fatal(err) + } + if _, err := s.Attach("s", Viewport{W: 640, H: 400, Scale: 1}); err != nil { + t.Fatal(err) + } + for _, c := range []struct{ id, title string }{{"one", "one a.png"}, {"many", "many a.png,b.png"}} { + if _, err := s.ClickAt(t.Context(), api.BrowserClickOptions{ID: "s", BrowserTarget: api.BrowserTarget{Selector: "#" + c.id}}); err != nil { + t.Fatal(err) + } + var title string + for deadline := time.Now().Add(10 * time.Second); time.Now().Before(deadline); time.Sleep(50 * time.Millisecond) { + info, err := s.Info("s") + if err != nil { + t.Fatal(err) + } + if title = info.Title; title == c.title { + break + } + } + if title != c.title { + t.Fatalf("#%s: title %q, want %q", c.id, title, c.title) + } + } +} diff --git a/internal/browser/files_test.go b/internal/browser/files_test.go new file mode 100644 index 000000000..edf4a479b --- /dev/null +++ b/internal/browser/files_test.go @@ -0,0 +1,47 @@ +package browser + +import ( + "errors" + "slices" + "testing" + + "github.com/chromedp/cdproto/page" +) + +func TestFileSinkPicksInTheInputsMode(t *testing.T) { + errPick := errors.New("no zenity") + for _, c := range []struct { + name string + watching bool + mode page.FileChooserOpenedMode + one string + many []string + err error + want []string + wantTitle string + }{ + {"one file", true, page.FileChooserOpenedModeSelectSingle, "/a.png", nil, nil, []string{"/a.png"}, chooseFileTitle}, + {"many files", true, page.FileChooserOpenedModeSelectMultiple, "", []string{"/a.png", "/b.png"}, nil, []string{"/a.png", "/b.png"}, chooseFilesTitle}, + {"canceled", true, page.FileChooserOpenedModeSelectSingle, "", nil, nil, nil, chooseFileTitle}, + {"picker failed", true, page.FileChooserOpenedModeSelectSingle, "", nil, errPick, nil, chooseFileTitle}, + {"no pane shows the page", false, page.FileChooserOpenedModeSelectSingle, "/a.png", nil, nil, nil, ""}, + } { + t.Run(c.name, func(t *testing.T) { + s := newService(nil, nil, nil, idleAfter) + var title string + s.SetPicker(func(tt string) (string, error) { title = tt; return c.one, c.err }, + func(tt string) ([]string, error) { title = tt; return c.many, c.err }) + got, err := s.fileSink(&session{watching: c.watching})(c.mode) + if !errors.Is(err, c.err) || !slices.Equal(got, c.want) || title != c.wantTitle { + t.Fatalf("got %v, %v under title %q; want %v, %v under %q", got, err, title, c.want, c.err, c.wantTitle) + } + }) + } +} + +func TestFileSinkWithoutAPickerLeavesTheInput(t *testing.T) { + s := newService(nil, nil, nil, idleAfter) + if got, err := s.fileSink(&session{watching: true})(page.FileChooserOpenedModeSelectSingle); got != nil || err != nil { + t.Fatalf("got %v, %v; want nothing", got, err) + } +} diff --git a/internal/browser/page.go b/internal/browser/page.go index 1c4501e8e..537c0b976 100644 --- a/internal/browser/page.go +++ b/internal/browser/page.go @@ -5,6 +5,7 @@ import ( "errors" "github.com/chromedp/cdproto/network" + "github.com/chromedp/cdproto/page" "github.com/lichdotdev/lich/api" ) @@ -129,6 +130,10 @@ type Page interface { // runs the page's script or input fails with errDialogOpen while one is // open. AnswerDialog(ctx context.Context, accept bool, text string) error + // OnFileChooser hands the page what to call when a file input asks for + // files, in the input's own mode; the paths it answers fill the input, + // and none leaves it as it was. + OnFileChooser(pick func(page.FileChooserOpenedMode) ([]string, error)) Screencast(ctx context.Context, v Viewport) error StopScreencast(ctx context.Context) error Resize(ctx context.Context, v Viewport) error diff --git a/internal/project/picker.go b/internal/project/picker.go index ce5d520c6..608d2bdf2 100644 --- a/internal/project/picker.go +++ b/internal/project/picker.go @@ -20,6 +20,14 @@ func (ZenityPicker) PickFile(title string) (string, error) { return mapZenity(zenity.SelectFile(zenity.Title(title))) } +func (ZenityPicker) PickFiles(title string) ([]string, error) { + paths, err := zenity.SelectFileMultiple(zenity.Title(title)) + if errors.Is(err, zenity.ErrCanceled) { + return nil, nil + } + return paths, err +} + // PickSaveFile opens the save dialog. ConfirmOverwrite is the dialog's own // guard: the caller writes whatever path comes back, so replacing an existing // file is agreed to here or nowhere. diff --git a/main.go b/main.go index 12faa974c..4e19725c7 100644 --- a/main.go +++ b/main.go @@ -226,6 +226,7 @@ func registerServices(db *store.Service, term *terminal.Service, svcs *services. dir, port := term.SessionCheckout(owner) return previews.URL(dir, port, raw) }) + brows.SetPicker(project.ZenityPicker{}.PickFile, project.ZenityPicker{}.PickFiles) proj := project.New(project.ZenityPicker{}) // gh has one active account per host; a project can name a different one. proj.SetAccounts(db.GHAccountForPath)