diff --git a/crates/ely_app/src/shell/chrome/topbar.rs b/crates/ely_app/src/shell/chrome/topbar.rs index 8a7fb72..5734e02 100644 --- a/crates/ely_app/src/shell/chrome/topbar.rs +++ b/crates/ely_app/src/shell/chrome/topbar.rs @@ -2,9 +2,9 @@ use ely_browser_core::BrowserSnapshot; use ely_design_system::{colors, spacing}; use ely_domain::{BrowserTab, ThemeMode}; use gpui::{ - AnyElement, BoxShadow, Context, FontWeight, InteractiveElement, IntoElement, ParentElement, - SharedString, StatefulInteractiveElement, Styled, div, hsla, point, prelude::FluentBuilder, px, - rgb, rgba, + AnyElement, BoxShadow, Context, Focusable, FontWeight, InteractiveElement, IntoElement, + ParentElement, SharedString, StatefulInteractiveElement, Styled, Window, div, hsla, point, + prelude::FluentBuilder, px, rgb, rgba, }; use gpui_component::{IconName, input::Input}; @@ -16,6 +16,7 @@ pub(crate) fn render_topbar( snapshot: &BrowserSnapshot, active_tab: &BrowserTab, sidebar_collapsed: bool, + window: &mut Window, cx: &mut Context, ) -> AnyElement { div() @@ -42,7 +43,7 @@ pub(crate) fn render_topbar( cx, |shell, window, cx| shell.navigate_active_tab_forward(window, cx), )) - .child(render_omnibar(shell, active_tab, cx)) + .child(render_omnibar(shell, active_tab, window, cx)) .child(render_topbar_action("share-url", IconName::Copy, cx, |shell, window, cx| { shell.copy_active_tab_url(window, cx) })) @@ -64,14 +65,14 @@ pub(crate) fn render_topbar( fn render_omnibar( shell: &mut ElyShell, active_tab: &BrowserTab, + window: &mut Window, cx: &mut Context, ) -> AnyElement { let favorite_active = active_tab.flags().favorite; let favorite_icon = if favorite_active { IconName::Star } else { IconName::StarOff }; - let input_value = shell.command_input.read(cx).value().to_string(); let active_url = active_tab.url().as_str().to_string(); - let show_styled = - !input_value.is_empty() && input_value == active_url && active_url != "ely://new-tab"; + let command_focused = shell.command_input.read(cx).focus_handle(cx).is_focused(window); + let show_styled = !command_focused && active_url != "ely://new-tab"; let secure = active_url.starts_with("https://") || active_url.starts_with("ely://"); div() diff --git a/crates/ely_app/src/shell/render.rs b/crates/ely_app/src/shell/render.rs index 4f7539d..e1ca2ff 100644 --- a/crates/ely_app/src/shell/render.rs +++ b/crates/ely_app/src/shell/render.rs @@ -17,10 +17,10 @@ use super::sidebar::collapsed_sidebar_active; use super::{ElyShell, ShellState}; impl Render for ElyShell { - fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { match &self.state { ShellState::Ready(core) => match (core.snapshot(), core.active_tab().cloned()) { - (Ok(snapshot), Ok(active_tab)) => self.render_browser(snapshot, active_tab, cx), + (Ok(snapshot), Ok(active_tab)) => self.render_browser(snapshot, active_tab, window, cx), (Err(error), _) | (_, Err(error)) => render_error(error.to_string()), }, ShellState::StartupError(message) => render_error(message.clone()), @@ -33,6 +33,7 @@ impl ElyShell { &mut self, snapshot: BrowserSnapshot, active_tab: BrowserTab, + window: &mut Window, cx: &mut Context, ) -> AnyElement { let sidebar_width = match active_sidebar_width(&snapshot) { @@ -88,7 +89,7 @@ impl ElyShell { sidebar_hidden, cx, )) - .child(self.render_main_pane(&snapshot, &active_tab, sidebar_collapsed, cx)), + .child(self.render_main_pane(&snapshot, &active_tab, sidebar_collapsed, window, cx)), ) .when(hover_expanded, |el| { el.child(self.render_hidden_sidebar_overlay(&snapshot, cx)) @@ -144,6 +145,7 @@ impl ElyShell { snapshot: &BrowserSnapshot, active_tab: &BrowserTab, sidebar_collapsed: bool, + window: &mut Window, cx: &mut Context, ) -> AnyElement { let panel_color = panel_bg(snapshot); @@ -159,7 +161,7 @@ impl ElyShell { .border_color(rgba(MAIN_PANE_HIGHLIGHT_BORDER)) .shadow(panel_shadow()) .overflow_hidden() - .child(render_topbar_chrome(self, snapshot, active_tab, sidebar_collapsed, cx)) + .child(render_topbar_chrome(self, snapshot, active_tab, sidebar_collapsed, window, cx)) .child( div() .flex_1() diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 18144c5..eade280 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -9,7 +9,7 @@ use super::{ web_surface_frame::WebSurfaceFrame, web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceSize}, web_surface_permissions::WebSurfaceSitePermission, - web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame}, + web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame, WebSurfaceUrlChange}, web_surface_state::{ PerTabSurface, WebSurfaceClickState, WebSurfaceInputOutcome, WebSurfaceKeyboardFocusState, WebSurfacePendingInput, WebSurfaceScrollState, WebSurfaceState, WebSurfaceTextInputState, @@ -39,58 +39,62 @@ impl WebSurfaceStore { tab: &BrowserTab, profile_data_mode: ProfileDataMode, permissions: &[WebSurfaceSitePermission], - ) { + ) -> Option { if !is_external_web_url(tab.url().as_str()) { - return; + return None; } let requested_url = tab.url().as_str().to_string(); - let Some(size) = self.surfaces.get(tab.id()).and_then(|surface| surface.viewport_size) - else { - return; - }; + let size = self.surfaces.get(tab.id()).and_then(|surface| surface.viewport_size)?; let input = self.take_pending_input(tab.id(), requested_url.as_str()); let previous_frame = self.previous_ready_frame(tab.id(), requested_url.as_str(), tab.zoom_percent()); match self.runtime.ensure_tab(tab, size, profile_data_mode, permissions, input) { Ok(result) if result.frame.is_some() => { + let url_change = result.url_change; let Some(frame) = result.frame else { - return; + return url_change; }; self.surface_mut(tab.id()).state = Some(WebSurfaceState::Ready(frame)); + url_change } Ok(result) if result.started_loading => { self.surface_mut(tab.id()).state = Some(WebSurfaceState::Loading { requested_url: result.requested_url, previous_frame, }); + result.url_change } - Ok(_) => {} + Ok(result) => result.url_change, Err(message) => { self.surface_mut(tab.id()).state = Some(WebSurfaceState::Failed { message }); + None } } } - pub(super) fn tick(&mut self) -> bool { + pub(super) fn tick(&mut self) -> WebSurfaceTickResult { let frames = self.runtime.tick(); - let mut changed = false; + let mut result = WebSurfaceTickResult::default(); for frame in frames { match frame { - WebSurfaceRuntimeFrame::Ready { tab_id, frame } => { - self.surface_mut(&tab_id).state = Some(WebSurfaceState::Ready(frame)); - changed = true; + WebSurfaceRuntimeFrame::Ready { tab_id, frame, url_change } => { + self.surface_mut(&tab_id).state = Some(WebSurfaceState::Ready(*frame)); + result.changed = true; + if let Some(url_change) = url_change { + result.url_changes.push(url_change); + } } WebSurfaceRuntimeFrame::Failed { tab_id, message } => { self.surface_mut(&tab_id).state = Some(WebSurfaceState::Failed { message }); - changed = true; + result.changed = true; } } } - changed + result } pub(super) fn record_scroll_delta( @@ -340,6 +344,12 @@ pub(super) fn is_external_web_url(url: &str) -> bool { url.starts_with("https://") || url.starts_with("http://") } +#[derive(Default)] +pub(super) struct WebSurfaceTickResult { + pub(super) changed: bool, + pub(super) url_changes: Vec, +} + #[cfg(test)] #[path = "web_surface_tests.rs"] mod tests; diff --git a/crates/ely_app/src/shell/web_surface_controller.rs b/crates/ely_app/src/shell/web_surface_controller.rs index 5a08b6d..f3d570b 100644 --- a/crates/ely_app/src/shell/web_surface_controller.rs +++ b/crates/ely_app/src/shell/web_surface_controller.rs @@ -1,5 +1,5 @@ use ely_browser_core::BrowserSnapshot; -use ely_domain::{BrowserTab, ProfileKind, TabId}; +use ely_domain::{BrowserTab, ProfileKind, TabId, UrlText}; use gpui::{AnyElement, Bounds, Context, Pixels, Point}; use crate::services::ProfileDataMode; @@ -7,6 +7,7 @@ use crate::services::ProfileDataMode; use super::{ ElyShell, web_surface_permissions::web_surface_site_permissions_for_tab, + web_surface_runtime::{WebSurfaceUrlChange, WebSurfaceUrlChangeKind}, web_surface_state::{WebSurfaceInputOutcome, WebSurfaceState}, web_surface_view::{ render_failed_web_surface, render_loading_web_surface, render_ready_web_surface, @@ -26,7 +27,12 @@ impl ElyShell { }; let permissions = web_surface_site_permissions_for_tab(tab, snapshot); - self.web_surfaces.ensure_surface(tab, profile_data_mode, &permissions); + if let Some(url_change) = + self.web_surfaces.ensure_surface(tab, profile_data_mode, &permissions) + && self.apply_web_surface_url_change(url_change) + { + cx.notify(); + } match self.web_surfaces.state(tab.id()) { Some(WebSurfaceState::Ready(frame)) => { @@ -45,7 +51,12 @@ impl ElyShell { } pub(super) fn tick_external_web_surfaces(&mut self) -> bool { - self.web_surfaces.tick() + let result = self.web_surfaces.tick(); + let mut url_changed = false; + for url_change in result.url_changes { + url_changed |= self.apply_web_surface_url_change(url_change); + } + result.changed || url_changed } pub(super) fn record_external_web_viewport( @@ -144,6 +155,26 @@ impl ElyShell { } } +impl ElyShell { + fn apply_web_surface_url_change(&mut self, change: WebSurfaceUrlChange) -> bool { + let Ok(url) = UrlText::parse(change.loaded_url) else { + return false; + }; + let super::ShellState::Ready(core) = &mut self.state else { + return false; + }; + + match change.kind { + WebSurfaceUrlChangeKind::UserInitiated => { + core.navigate_tab_to_loaded_url(&change.tab_id, url).is_ok_and(|changed| changed) + } + WebSurfaceUrlChangeKind::Observed => { + core.replace_tab_loaded_url(&change.tab_id, url).is_ok_and(|changed| changed) + } + } + } +} + fn profile_data_mode_for(tab: &BrowserTab, snapshot: &BrowserSnapshot) -> Option { snapshot.profiles.iter().find(|profile| profile.id() == tab.profile_id()).map(|profile| { match profile.kind() { diff --git a/crates/ely_app/src/shell/web_surface_frame.rs b/crates/ely_app/src/shell/web_surface_frame.rs index 97d11b8..931168f 100644 --- a/crates/ely_app/src/shell/web_surface_frame.rs +++ b/crates/ely_app/src/shell/web_surface_frame.rs @@ -200,6 +200,10 @@ impl WebSurfaceFrame { self.zoom_percent } + pub(super) fn loaded_url(&self) -> Option<&str> { + self.loaded_url.as_deref() + } + #[cfg(all(test, feature = "live-site-smoke"))] pub(super) fn click_point(&self) -> Option { self.click_point diff --git a/crates/ely_app/src/shell/web_surface_runtime.rs b/crates/ely_app/src/shell/web_surface_runtime.rs index 29d9120..d141446 100644 --- a/crates/ely_app/src/shell/web_surface_runtime.rs +++ b/crates/ely_app/src/shell/web_surface_runtime.rs @@ -47,10 +47,17 @@ impl WebSurfaceRuntime { } let (scroll_delta_x, scroll_delta_y, scroll_point_x, scroll_point_y) = scroll_wire_fields(input.scroll_delta, input.scroll_point)?; + let user_navigation_input = input_requests_history_navigation(&input); let session = sessions.entry(tab.id().clone()).or_insert_with(WebSurfaceSession::default); let next_scroll_offset = input.scroll_offset; let started_loading = session.started_loading(&requested_url, size, zoom_percent); + if started_loading { + session.pending_user_navigation = false; + } + if user_navigation_input { + session.pending_user_navigation = true; + } let frame = client .ensure(ServoLiveEnsureRequest { tab_id: tab.id().as_str().to_string(), @@ -82,17 +89,20 @@ impl WebSurfaceRuntime { .map_err(|error| error.to_string()) }) .transpose()?; + let url_change = frame + .as_ref() + .and_then(|frame| session.url_change_for(tab.id(), requested_url.as_str(), frame)); session.requested_url = requested_url.clone(); session.size = size; session.zoom_percent = zoom_percent; session.scroll_offset = next_scroll_offset; - Ok(WebSurfaceEnsureResult { requested_url, started_loading, frame }) + Ok(WebSurfaceEnsureResult { requested_url, started_loading, frame, url_change }) } pub(super) fn tick(&mut self) -> Vec { - let (state, sessions) = (&mut self.state, &self.sessions); + let (state, sessions) = (&mut self.state, &mut self.sessions); let RuntimeState::Ready { client, .. } = state else { return Vec::new(); }; @@ -107,7 +117,14 @@ impl WebSurfaceRuntime { frame, ) { Ok(frame) => { - frames.push(WebSurfaceRuntimeFrame::Ready { tab_id: tab_id.clone(), frame }) + let requested_url = session.requested_url.clone(); + let url_change = + session.url_change_for(tab_id, requested_url.as_str(), &frame); + frames.push(WebSurfaceRuntimeFrame::Ready { + tab_id: tab_id.clone(), + frame: Box::new(frame), + url_change, + }) } Err(error) => frames.push(WebSurfaceRuntimeFrame::Failed { tab_id: tab_id.clone(), @@ -185,6 +202,7 @@ struct WebSurfaceSession { size: WebSurfaceSize, zoom_percent: u16, scroll_offset: WebSurfaceScrollOffset, + pending_user_navigation: bool, } impl WebSurfaceSession { @@ -198,19 +216,57 @@ impl WebSurfaceSession { || self.size != size || self.zoom_percent != zoom_percent } + + fn url_change_for( + &mut self, + tab_id: &TabId, + requested_url: &str, + frame: &WebSurfaceFrame, + ) -> Option { + let loaded_url = frame.loaded_url()?; + if loaded_url == requested_url { + return None; + } + + let kind = if self.pending_user_navigation { + WebSurfaceUrlChangeKind::UserInitiated + } else { + WebSurfaceUrlChangeKind::Observed + }; + self.pending_user_navigation = false; + Some(WebSurfaceUrlChange { + tab_id: tab_id.clone(), + loaded_url: loaded_url.to_string(), + kind, + }) + } } pub(super) struct WebSurfaceEnsureResult { pub(super) requested_url: String, pub(super) started_loading: bool, pub(super) frame: Option, + pub(super) url_change: Option, } pub(super) enum WebSurfaceRuntimeFrame { - Ready { tab_id: TabId, frame: WebSurfaceFrame }, + Ready { tab_id: TabId, frame: Box, url_change: Option }, Failed { tab_id: TabId, message: String }, } +#[derive(Clone, Debug, Eq, PartialEq)] +pub(super) struct WebSurfaceUrlChange { + pub(super) tab_id: TabId, + pub(super) loaded_url: String, + pub(super) kind: WebSurfaceUrlChangeKind, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum WebSurfaceUrlChangeKind { + UserInitiated, + Observed, +} + fn config_dir_for_scope( scope: &WebSurfaceRuntimeScope, ) -> Result<(PathBuf, Option), String> { @@ -245,6 +301,11 @@ fn scroll_wire_fields( } } +fn input_requests_history_navigation(input: &WebSurfacePendingInput) -> bool { + input.click_point.is_some() + || input.typed_text.as_deref().is_some_and(|text| text.contains('\n')) +} + impl From<&WebSurfaceSitePermission> for ServoLiveSitePermission { fn from(permission: &WebSurfaceSitePermission) -> Self { Self::new( diff --git a/crates/ely_browser_core/src/state/tabs.rs b/crates/ely_browser_core/src/state/tabs.rs index 071236a..c9225cb 100644 --- a/crates/ely_browser_core/src/state/tabs.rs +++ b/crates/ely_browser_core/src/state/tabs.rs @@ -23,19 +23,26 @@ impl BrowserCore { /// no new tab is created and the active tab id is unchanged. pub fn navigate_active_tab(&mut self, url: UrlText) -> Result<(), CoreError> { let active_id = self.active_tab_id.clone(); - let tab_index = self - .tabs - .iter() - .position(|tab| tab.id() == &active_id) - .ok_or_else(|| CoreError::TabNotFound { id: active_id.clone() })?; - self.tabs[tab_index].navigate_to(url); - self.tabs[tab_index].mark_ready(); - let snapshot_tab = self.tabs[tab_index].clone(); - self.record_history_entry(&snapshot_tab); - self.record_tab_activity(&active_id, SystemTime::now()); + self.update_tab_url(&active_id, url, TabUrlUpdate::PushHistory)?; Ok(()) } + pub fn navigate_tab_to_loaded_url( + &mut self, + tab_id: &TabId, + url: UrlText, + ) -> Result { + self.update_tab_url(tab_id, url, TabUrlUpdate::PushHistory) + } + + pub fn replace_tab_loaded_url( + &mut self, + tab_id: &TabId, + url: UrlText, + ) -> Result { + self.update_tab_url(tab_id, url, TabUrlUpdate::PreserveHistory) + } + pub fn navigate_active_tab_back(&mut self) -> Result { self.navigate_active_tab_history(TabHistoryDirection::Back) } @@ -377,6 +384,32 @@ impl BrowserCore { self.tabs.get_mut(active_index).ok_or(CoreError::MissingActiveTab) } + fn update_tab_url( + &mut self, + tab_id: &TabId, + url: UrlText, + update: TabUrlUpdate, + ) -> Result { + let tab_index = self + .tabs + .iter() + .position(|tab| tab.id() == tab_id) + .ok_or_else(|| CoreError::TabNotFound { id: tab_id.clone() })?; + if self.tabs[tab_index].url() == &url { + return Ok(false); + } + + match update { + TabUrlUpdate::PushHistory => self.tabs[tab_index].navigate_to(url), + TabUrlUpdate::PreserveHistory => self.tabs[tab_index].set_url(url), + } + self.tabs[tab_index].mark_ready(); + let snapshot_tab = self.tabs[tab_index].clone(); + self.record_history_entry(&snapshot_tab); + self.record_tab_activity(tab_id, SystemTime::now()); + Ok(true) + } + fn navigate_active_tab_history( &mut self, direction: TabHistoryDirection, @@ -459,3 +492,8 @@ enum TabHistoryDirection { Back, Forward, } + +enum TabUrlUpdate { + PushHistory, + PreserveHistory, +} diff --git a/crates/ely_browser_core/tests/tab_navigation.rs b/crates/ely_browser_core/tests/tab_navigation.rs index 567d049..c10ee6f 100644 --- a/crates/ely_browser_core/tests/tab_navigation.rs +++ b/crates/ely_browser_core/tests/tab_navigation.rs @@ -40,6 +40,34 @@ fn new_navigation_clears_forward_stack() -> Result<(), Box> { Ok(()) } +#[test] +fn observed_loaded_url_replaces_active_url_without_back_stack() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let tab_id = core.active_tab()?.id().clone(); + + assert!(core.replace_tab_loaded_url(&tab_id, UrlText::parse("https://example.com/")?)?); + + assert_eq!(core.active_tab()?.url().as_str(), "https://example.com/"); + assert!(!core.active_tab()?.can_navigate_back()); + assert!(!core.active_tab()?.can_navigate_forward()); + Ok(()) +} + +#[test] +fn user_loaded_url_enters_active_back_stack() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + core.navigate_active_tab(UrlText::parse("https://example.com/a")?)?; + let tab_id = core.active_tab()?.id().clone(); + + assert!(core.navigate_tab_to_loaded_url(&tab_id, UrlText::parse("https://example.com/b")?)?); + + assert_eq!(core.active_tab()?.url().as_str(), "https://example.com/b"); + assert!(core.active_tab()?.can_navigate_back()); + assert!(core.navigate_active_tab_back()?); + assert_eq!(core.active_tab()?.url().as_str(), "https://example.com/a"); + Ok(()) +} + #[test] fn empty_history_navigation_keeps_active_url() -> Result<(), Box> { let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;