diff --git a/Cargo.lock b/Cargo.lock index 34202fc..f9a5ac8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3125,6 +3125,19 @@ version = "0.32.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e629b9b98ef3dd8afe6ca2bd0f89306cec16d43d907889945bc5d6687f2f13c7" +[[package]] +name = "git2" +version = "0.20.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b88256088d75a56f8ecfa070513a775dd9107f6530ef14919dac831af9cfe2b" +dependencies = [ + "bitflags 2.11.1", + "libc", + "libgit2-sys", + "log", + "url", +] + [[package]] name = "gl_generator" version = "0.14.0" @@ -3289,6 +3302,7 @@ dependencies = [ "as-raw-xcb-connection", "ashpd 0.11.1", "async-task", + "backtrace", "bindgen 0.71.1", "blade-graphics", "blade-macros", @@ -3567,12 +3581,15 @@ dependencies = [ "dunce", "futures", "futures-lite 1.13.0", + "git2", "globset", "gpui_collections", + "gpui_util_macros", "itertools 0.14.0", "libc", "log", "nix 0.29.0", + "rand 0.9.4", "regex", "rust-embed", "schemars", @@ -5122,6 +5139,18 @@ dependencies = [ "cc", ] +[[package]] +name = "libgit2-sys" +version = "0.18.4+1.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b26f66f35e1871b22efcf7191564123d2a446ca0538cde63c23adfefa9b15b7" +dependencies = [ + "cc", + "libc", + "libz-sys", + "pkg-config", +] + [[package]] name = "libloading" version = "0.8.9" diff --git a/crates/ely_app/Cargo.toml b/crates/ely_app/Cargo.toml index c654ace..8ca27fb 100644 --- a/crates/ely_app/Cargo.toml +++ b/crates/ely_app/Cargo.toml @@ -25,6 +25,9 @@ thiserror.workspace = true ureq.workspace = true url.workspace = true +[dev-dependencies] +gpui = { workspace = true, features = ["test-support"] } + [build-dependencies] toml.workspace = true diff --git a/crates/ely_app/src/shell/gpui_harness_tests.rs b/crates/ely_app/src/shell/gpui_harness_tests.rs new file mode 100644 index 0000000..279fcfd --- /dev/null +++ b/crates/ely_app/src/shell/gpui_harness_tests.rs @@ -0,0 +1,197 @@ +//! GPUI test harness for the input pipeline. +//! +//! Twelve sidecar-side commits and one shell-side commit had all claimed to +//! fix "click does nothing" while the user kept reporting the same symptom. +//! The roundtable consensus: every store-layer test passed GREEN, every +//! sidecar integration test passed GREEN, but nothing in the repo exercised +//! the real GPUI event tree (`render_input_overlay` + window-level mouse +//! handlers + sidebar capture interactions). This module is that missing +//! holdout set. +//! +//! What we have so far: +//! 1. `baseline_overlay_div_receives_simulated_click` — proves GPUI's +//! `.occlude()` + `capture_any_mouse_up` primitive works correctly in +//! `TestAppContext`. If this regresses, the harness itself is broken. +//! 2. `baseline_overlay_with_full_listener_combo_receives_click` — proves +//! the exact listener combo `render_input_overlay` uses (on_mouse_down + +//! capture_any_mouse_up + on_mouse_move + on_scroll_wheel on a single +//! `.occlude()` div) works in isolation. +//! 3. `ely_shell_external_canvas_lays_out_inside_window` — boots a real +//! `ElyShell`, navigates to an external URL, and asserts the +//! input_overlay's measured viewport bounds fit inside the visible +//! window. This was the original ship-blocker: the overlay was being +//! positioned at `y = window_height - 17`, entirely below the visible +//! region, so every user click hit empty space above the overlay. + +use std::cell::RefCell; +use std::rc::Rc; + +use ely_domain::{TabId, UrlText}; +use gpui::{ + Bounds, Context, IntoElement, Modifiers, MouseButton, ParentElement, Pixels, Render, + Styled, TestAppContext, Window, div, point, px, +}; +use gpui::InteractiveElement; + +use super::ShellState; +use super::web_surface::WebSurfaceStore; + +#[cfg(test)] +impl super::ElyShell { + pub(super) fn web_surfaces_for_test(&self) -> &WebSurfaceStore { + &self.web_surfaces + } +} + +#[gpui::test] +async fn ely_shell_external_canvas_lays_out_inside_window(cx: &mut TestAppContext) { + cx.update(|cx| gpui_component::init(cx)); + + let (shell, cx) = cx.add_window_view(|window, cx| super::ElyShell::new(window, cx)); + cx.run_until_parked(); + + cx.update(|window, app_cx| { + shell.update(app_cx, |shell, ctx| { + shell.navigate_active_tab( + UrlText::parse("https://example.com/".to_string()).expect("valid URL"), + window, + ctx, + ); + }); + }); + cx.run_until_parked(); + + let (_active_tab_id, active_tab_url, viewport_bounds) = active_tab_overlay_state(&shell, cx); + assert!( + active_tab_url.starts_with("https://"), + "active tab URL must be external https for render_external_web_canvas \ + to render the input_overlay (got {active_tab_url:?})." + ); + let bounds = viewport_bounds.unwrap_or_else(|| { + panic!( + "viewport_bounds for the active tab is None. The canvas tracker \ + in render_input_overlay's sibling never fired its layout \ + callback — render_external_web_canvas was not reached." + ) + }); + let window_size = cx.update(|window, _| window.bounds().size); + assert!( + bounds.origin.y + bounds.size.height <= window_size.height + px(1.0) + && bounds.origin.x + bounds.size.width <= window_size.width + px(1.0), + "Layout regression: input_overlay viewport_bounds {bounds:?} extend \ + outside the {window_size:?} window. The canvas tracker measured a \ + layout that escapes the visible region — every user click in the \ + visible area now lands above (or beside) the overlay's hitbox. \ + The original ship-blocker was bounds.origin.y == window_height-17 \ + caused by content (the rendered web image) being a non-absolute \ + child of the relative wrapper, which doubled the parent's height \ + and pushed the overlay off-screen." + ); +} + +#[gpui::test] +async fn baseline_overlay_with_full_listener_combo_receives_click( + cx: &mut TestAppContext, +) { + let click_count = Rc::new(RefCell::new(0u32)); + let counter_for_render = click_count.clone(); + + struct ComboProbe { + on_up_counter: Rc>, + } + impl Render for ComboProbe { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + let counter = self.on_up_counter.clone(); + div().relative().size_full().child( + div() + .absolute() + .size_full() + .occlude() + .on_mouse_down(MouseButton::Left, |_event, _window, _cx| {}) + .capture_any_mouse_up(move |_event, _window, _cx| { + *counter.borrow_mut() += 1; + }) + .on_mouse_move(|_event, _window, _cx| {}) + .on_scroll_wheel(|_event, _window, _cx| {}), + ) + } + } + + let (_probe, cx) = cx.add_window_view(|_window, _cx| ComboProbe { + on_up_counter: counter_for_render, + }); + cx.run_until_parked(); + + cx.simulate_mouse_move(point(px(100.0), px(100.0)), None, Modifiers::default()); + cx.simulate_click(point(px(100.0), px(100.0)), Modifiers::default()); + cx.run_until_parked(); + + assert_eq!( + *click_count.borrow(), + 1, + "GPUI baseline with input_overlay's full listener combo: click should \ + reach capture_any_mouse_up. If this fails, the listener combo itself \ + is the problem, not the surrounding shell." + ); +} + +#[gpui::test] +async fn baseline_overlay_div_receives_simulated_click(cx: &mut TestAppContext) { + let click_count = Rc::new(RefCell::new(0u32)); + let counter_for_render = click_count.clone(); + + struct Probe { + on_up_counter: Rc>, + } + impl Render for Probe { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + let counter = self.on_up_counter.clone(); + div().relative().size_full().child( + div().absolute().size_full().occlude().capture_any_mouse_up( + move |_event, _window, _cx| { + *counter.borrow_mut() += 1; + }, + ), + ) + } + } + + let (_probe, cx) = cx.add_window_view(|_window, _cx| Probe { + on_up_counter: counter_for_render, + }); + cx.run_until_parked(); + + cx.simulate_mouse_move(point(px(100.0), px(100.0)), None, Modifiers::default()); + cx.simulate_click(point(px(100.0), px(100.0)), Modifiers::default()); + cx.run_until_parked(); + + assert_eq!( + *click_count.borrow(), + 1, + "GPUI baseline: a div with .absolute().size_full().occlude() and \ + capture_any_mouse_up never received the simulated click. The test \ + harness or GPUI primitive is broken — ElyShell test results are \ + meaningless until this passes." + ); +} + +fn active_tab_overlay_state( + shell: &gpui::Entity, + cx: &mut gpui::VisualTestContext, +) -> (TabId, String, Option>) { + shell.read_with(cx, |shell, _cx| { + let tab = match &shell.state { + ShellState::Ready(core) => core.active_tab().expect("active tab exists"), + ShellState::StartupError(message) => { + panic!("ElyShell failed to start in test: {message}") + } + }; + let tab_id = tab.id().clone(); + let url = tab.url().as_str().to_string(); + let bounds = shell + .web_surfaces_for_test() + .surface_for_test(&tab_id) + .and_then(|surface| surface.viewport_bounds); + (tab_id, url, bounds) + }) +} diff --git a/crates/ely_app/src/shell/mod.rs b/crates/ely_app/src/shell/mod.rs index 58c960c..363a435 100644 --- a/crates/ely_app/src/shell/mod.rs +++ b/crates/ely_app/src/shell/mod.rs @@ -33,6 +33,9 @@ mod web_surface_runtime; mod web_surface_state; mod web_surface_view; +#[cfg(test)] +mod gpui_harness_tests; + use std::time::Duration; use ely_browser_core::{BrowserCore, InitialBrowserConfig}; diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 304d2f2..9dd3211 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -315,6 +315,11 @@ impl WebSurfaceStore { fn surface_mut(&mut self, tab_id: &TabId) -> &mut PerTabSurface { self.surfaces.entry(tab_id.clone()).or_insert_with(PerTabSurface::new) } + + #[cfg(test)] + pub(super) fn surface_for_test(&self, tab_id: &TabId) -> Option<&PerTabSurface> { + self.surfaces.get(tab_id) + } } pub(super) fn is_external_web_url(url: &str) -> bool { diff --git a/crates/ely_app/src/shell/web_surface_tests.rs b/crates/ely_app/src/shell/web_surface_tests.rs index 9457071..9f13c86 100644 --- a/crates/ely_app/src/shell/web_surface_tests.rs +++ b/crates/ely_app/src/shell/web_surface_tests.rs @@ -213,6 +213,120 @@ fn zero_wheel_delta_reports_zero_delta() -> Result<(), Box> { Ok(()) } +/// Pinning the per-tab isolation invariant. A click recorded against +/// tab A must not be drained by, dropped by, or overwritten by any +/// state mutation routed to tab B. The store keys every click on its +/// owning `TabId` via `PerTabSurface`, but the singleton +/// `keyboard_focus` cross-cuts tabs — so a regression that +/// accidentally entangled them (e.g. dropping A's `click_point` when +/// B took focus) would surface here. +#[test] +fn click_on_tab_a_survives_click_on_tab_b() -> Result<(), Box> { + let mut store = WebSurfaceStore::new(); + let tab_a = web_tab("https://example.com/a")?; + let tab_b = web_tab("https://example.com/b")?; + + assert_applied(store.record_viewport_size(tab_a.id(), web_bounds(), 1.0)); + assert_applied(store.record_viewport_size(tab_b.id(), web_bounds(), 1.0)); + + assert_applied(store.record_click_point( + tab_a.id(), + tab_a.url().as_str(), + point(px(40.0), px(40.0)), + 1.0, + )); + assert_applied(store.record_click_point( + tab_b.id(), + tab_b.url().as_str(), + point(px(200.0), px(200.0)), + 1.0, + )); + + let input_a = store.take_pending_input(tab_a.id(), tab_a.url().as_str()); + assert_eq!( + input_a.click_point.map(|p| (p.x(), p.y())), + Some((40, 40)), + "tab A's click must survive a subsequent click on tab B — \ + per-tab surfaces are independent owners of `click_point`", + ); + + let input_b = store.take_pending_input(tab_b.id(), tab_b.url().as_str()); + assert_eq!( + input_b.click_point.map(|p| (p.x(), p.y())), + Some((200, 200)), + "tab B's click must drain into B's pending input, not A's", + ); + Ok(()) +} + +/// Pinning the zero-delta short-circuit's non-effect on a buffered +/// click. `record_scroll_delta` wipes `click_point` (post-scroll +/// coords would target the wrong DOM node), but the early `None` +/// return for `DroppedZeroDelta` must short-circuit *before* the wipe. +/// A future refactor that moved the wipe above the delta check would +/// silently eat clicks whenever a precision-mouse wheel reported a +/// sub-device-pixel delta. +#[test] +fn zero_wheel_delta_must_not_erase_buffered_click() -> Result<(), Box> { + let mut store = WebSurfaceStore::new(); + let tab = web_tab("https://example.com/form")?; + let url = tab.url().as_str(); + + assert_applied(store.record_viewport_size(tab.id(), web_bounds(), 1.0)); + assert_applied(store.record_click_point(tab.id(), url, point(px(160.0), px(120.0)), 1.0)); + + assert_eq!( + store.record_scroll_delta(tab.id(), url, point(px(0.0), px(0.0)), 1.0), + WebSurfaceInputOutcome::DroppedZeroDelta, + ); + + let input = store.take_pending_input(tab.id(), url); + assert_eq!( + input.click_point.map(|p| (p.x(), p.y())), + Some((160, 120)), + "a zero-delta wheel event reports DroppedZeroDelta and must not \ + take the `click_point` wipe path — the early return guards it", + ); + Ok(()) +} + +/// Pinning the bounds-vs-drain decoupling. A viewport resize between +/// click and drain leaves `viewport_bounds` mutated but does not +/// touch `click_point`, `scroll_offset`, or `requested_url` — the +/// three keys the drain filter checks. The click's stored device-px +/// coordinates remain valid against the new bounds because GPUI +/// re-renders before any new click can arrive. +/// +/// If a future refactor stored raw window-relative coords on +/// `click_point` and converted them at drain time, a resize between +/// record and drain would shift the result; this test would still +/// drain "something" but the coordinates would change, surfacing the +/// drift. +#[test] +fn click_survives_viewport_bounds_change_before_drain() -> Result<(), Box> { + let mut store = WebSurfaceStore::new(); + let tab = web_tab("https://example.com/resize")?; + let url = tab.url().as_str(); + + assert_applied(store.record_viewport_size(tab.id(), web_bounds(), 1.0)); + assert_applied(store.record_click_point(tab.id(), url, point(px(160.0), px(120.0)), 1.0)); + + // Resize: first measurement is buffered (requires confirmation). + assert_eq!( + store.record_viewport_size(tab.id(), resized_once_bounds(), 1.0), + WebSurfaceInputOutcome::Buffered, + ); + + let input = store.take_pending_input(tab.id(), url); + assert_eq!( + input.click_point.map(|p| (p.x(), p.y())), + Some((160, 120)), + "a resize-in-progress must not steal the buffered click — \ + the drain filter checks url+scroll_offset, not bounds", + ); + Ok(()) +} + fn web_bounds() -> Bounds { Bounds::new(point(px(0.0), px(0.0)), size(px(640.0), px(480.0))) } diff --git a/crates/ely_app/src/shell/web_surface_view.rs b/crates/ely_app/src/shell/web_surface_view.rs index 11f20ab..7380923 100644 --- a/crates/ely_app/src/shell/web_surface_view.rs +++ b/crates/ely_app/src/shell/web_surface_view.rs @@ -69,19 +69,18 @@ fn render_web_surface( let tracker_entity = state_entity; div() - .flex_1() - .h_full() + .relative() + .size_full() .min_w_0() .overflow_hidden() .child( div() - .relative() - .size_full() - .overflow_hidden() - .child(content) - .child(render_viewport_tracker(tab.id().clone(), tracker_entity)) - .child(render_input_overlay(input_tab_id, input_url, input_entity)), + .absolute() + .inset_0() + .child(content), ) + .child(render_viewport_tracker(tab.id().clone(), tracker_entity)) + .child(render_input_overlay(input_tab_id, input_url, input_entity)) .into_any_element() }