Propagate page title and favicon into the active tab
Servo already publishes the live page title in every `LiveFrameReport` but the renderer was dropping it on the floor — tabs that navigated away from `ely://new-tab` kept showing "New Tab" forever, and there was no favicon visible anywhere in the sidebar. Add `BrowserCore::set_tab_title` and switch `set_tab_favicon_key` to return `Ok(true)` only when the value actually changed; both methods mirror the new value into the matching history entry so the History page stays in lockstep. Derive the canonical `/favicon.ico` URL from the loaded URL on `UrlText` and store it as the tab's `favicon_key`. In the surface layer, every Ready frame now emits a `WebSurfacePageMetadata` change alongside any `WebSurfaceUrlChange`, and the controller applies title + favicon URL together. Render the sidebar tab row's favicon via GPUI's HTTP image loader (falling through to the URL-derived glyph for `ely://` pages, file URLs, and hosts without a /favicon.ico endpoint).
This commit is contained in:
@@ -2,9 +2,9 @@ use ely_browser_core::BrowserSnapshot;
|
||||
use ely_design_system::{colors, spacing};
|
||||
use ely_domain::BrowserTab;
|
||||
use gpui::{
|
||||
AnyElement, Context, FontWeight, InteractiveElement, IntoElement, ParentElement, SharedString,
|
||||
StatefulInteractiveElement, Styled, div, hsla, linear_color_stop, linear_gradient,
|
||||
prelude::FluentBuilder, px, rgb, rgba,
|
||||
AnyElement, Context, FontWeight, ImageSource, InteractiveElement, IntoElement, ObjectFit,
|
||||
ParentElement, SharedString, StatefulInteractiveElement, Styled, StyledImage, div, hsla, img,
|
||||
linear_color_stop, linear_gradient, prelude::FluentBuilder, px, rgb, rgba,
|
||||
};
|
||||
use gpui_component::{IconName, StyledExt, scroll::ScrollableElement};
|
||||
|
||||
@@ -314,6 +314,8 @@ impl ElyShell {
|
||||
let palette = nav_row_palette(active);
|
||||
let group_name = SharedString::from(format!("tab-{}", tab.id().as_str()));
|
||||
let close_id = SharedString::from(format!("tab-close-{}", tab.id().as_str()));
|
||||
let title = tab.title().to_string();
|
||||
let initial = title.chars().next().unwrap_or('?').to_string();
|
||||
|
||||
div()
|
||||
.id(SharedString::from(tab.id().as_str().to_string()))
|
||||
@@ -332,6 +334,7 @@ impl ElyShell {
|
||||
.on_click(cx.listener(move |shell, _, window, cx| {
|
||||
shell.select_tab(&tab_id, window, cx);
|
||||
}))
|
||||
.child(render_tab_favicon(tab, &initial))
|
||||
.child(
|
||||
div()
|
||||
.flex_1()
|
||||
@@ -419,3 +422,31 @@ where
|
||||
.on_click(on_click)
|
||||
.child(IconName::Close)
|
||||
}
|
||||
|
||||
/// Resolve the favicon glyph for a tab row. Prefers the favicon URL
|
||||
/// the Servo runtime derived from the loaded URL; falls back to the
|
||||
/// initial-letter chip used everywhere else when the tab has no live
|
||||
/// favicon (yet to load, internal page, file URL, etc.).
|
||||
fn render_tab_favicon(tab: &BrowserTab, initial: &str) -> AnyElement {
|
||||
if let Some(favicon_url) = tab.favicon_key()
|
||||
&& favicon_url.starts_with("http")
|
||||
{
|
||||
return div()
|
||||
.size(px(FAVICON_SIZE))
|
||||
.flex_shrink_0()
|
||||
.rounded(px(FAVICON_RADIUS))
|
||||
.overflow_hidden()
|
||||
.child(
|
||||
img(ImageSource::from(favicon_url.to_string()))
|
||||
.size(px(FAVICON_SIZE))
|
||||
.object_fit(ObjectFit::Cover),
|
||||
)
|
||||
.into_any_element();
|
||||
}
|
||||
|
||||
let host = tab.url().host();
|
||||
render_glyph_for(host.as_deref(), initial, FAVICON_SIZE)
|
||||
}
|
||||
|
||||
const FAVICON_SIZE: f32 = 16.0;
|
||||
const FAVICON_RADIUS: f32 = 4.0;
|
||||
|
||||
@@ -17,6 +17,33 @@ use super::{
|
||||
},
|
||||
};
|
||||
|
||||
/// One page's worth of metadata observed in a Ready frame. The
|
||||
/// controller applies these to the `BrowserTab` (title / favicon_key)
|
||||
/// after the frame has been swapped into the surface state. Title and
|
||||
/// favicon are independent — a navigation typically settles the URL
|
||||
/// first, then Servo emits a title change a frame or two later, and
|
||||
/// the favicon URL is derived from the loaded URL.
|
||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||
pub(super) struct WebSurfacePageMetadata {
|
||||
pub(super) tab_id: TabId,
|
||||
pub(super) title: Option<String>,
|
||||
pub(super) favicon_url: Option<String>,
|
||||
}
|
||||
|
||||
impl WebSurfacePageMetadata {
|
||||
fn from_frame(tab_id: &TabId, frame: &WebSurfaceFrame) -> Option<Self> {
|
||||
let title = frame.title().map(str::to_string);
|
||||
let favicon_url = frame
|
||||
.loaded_url()
|
||||
.and_then(|loaded| ely_domain::UrlText::parse(loaded).ok())
|
||||
.and_then(|url| url.favicon_url());
|
||||
if title.is_none() && favicon_url.is_none() {
|
||||
return None;
|
||||
}
|
||||
Some(Self { tab_id: tab_id.clone(), title, favicon_url })
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) struct WebSurfaceStore {
|
||||
runtime: WebSurfaceRuntime,
|
||||
/// Single owner of every per-tab invariant. See [`PerTabSurface`].
|
||||
@@ -108,6 +135,9 @@ impl WebSurfaceStore {
|
||||
result.changed = true;
|
||||
continue;
|
||||
}
|
||||
if let Some(metadata) = WebSurfacePageMetadata::from_frame(&tab_id, &frame) {
|
||||
result.page_metadata.push(metadata);
|
||||
}
|
||||
self.surface_mut(&tab_id).state = Some(WebSurfaceState::Ready(*frame));
|
||||
result.changed = true;
|
||||
if let Some(url_change) = url_change {
|
||||
@@ -441,6 +471,7 @@ pub(super) struct WebSurfaceEnsureOutcome {
|
||||
pub(super) struct WebSurfaceTickResult {
|
||||
pub(super) changed: bool,
|
||||
pub(super) url_changes: Vec<WebSurfaceUrlChange>,
|
||||
pub(super) page_metadata: Vec<WebSurfacePageMetadata>,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::services::ProfileDataMode;
|
||||
|
||||
use super::{
|
||||
ElyShell,
|
||||
web_surface::WebSurfacePageMetadata,
|
||||
web_surface_permissions::web_surface_site_permissions_for_tab,
|
||||
web_surface_runtime::{WebSurfaceUrlChange, WebSurfaceUrlChangeKind},
|
||||
web_surface_state::{WebSurfaceInputOutcome, WebSurfaceState},
|
||||
@@ -60,7 +61,11 @@ impl ElyShell {
|
||||
for url_change in result.url_changes {
|
||||
url_changed |= self.apply_web_surface_url_change(url_change);
|
||||
}
|
||||
result.changed || url_changed
|
||||
let mut metadata_changed = false;
|
||||
for metadata in result.page_metadata {
|
||||
metadata_changed |= self.apply_web_surface_page_metadata(metadata);
|
||||
}
|
||||
result.changed || url_changed || metadata_changed
|
||||
}
|
||||
|
||||
pub(super) fn record_external_web_viewport(
|
||||
@@ -184,6 +189,24 @@ impl ElyShell {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn apply_web_surface_page_metadata(&mut self, metadata: WebSurfacePageMetadata) -> bool {
|
||||
let super::ShellState::Ready(core) = &mut self.state else {
|
||||
return false;
|
||||
};
|
||||
let mut changed = false;
|
||||
if let Some(title) = metadata.title
|
||||
&& let Ok(true) = core.set_tab_title(&metadata.tab_id, title)
|
||||
{
|
||||
changed = true;
|
||||
}
|
||||
if let Some(favicon_url) = metadata.favicon_url
|
||||
&& let Ok(true) = core.set_tab_favicon_key(&metadata.tab_id, favicon_url)
|
||||
{
|
||||
changed = true;
|
||||
}
|
||||
changed
|
||||
}
|
||||
}
|
||||
|
||||
fn visible_external_web_tabs<'a>(
|
||||
|
||||
@@ -228,6 +228,10 @@ impl WebSurfaceFrame {
|
||||
self.loaded_url.as_deref()
|
||||
}
|
||||
|
||||
pub(super) fn title(&self) -> Option<&str> {
|
||||
self.title.as_deref()
|
||||
}
|
||||
|
||||
pub(super) fn has_visible_content_for_initial_display(&self) -> Result<bool, WebSurfaceError> {
|
||||
#[cfg(target_os = "macos")]
|
||||
if let Some(pixel_buffer) = self.pixel_buffer.as_ref() {
|
||||
|
||||
@@ -84,6 +84,17 @@ impl BrowserCore {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn set_history_title_for_tab(&mut self, tab: &BrowserTab, title: impl Into<String>) {
|
||||
let title = title.into();
|
||||
if let Some(entry) = self.history_entries.iter_mut().find(|entry| {
|
||||
entry.profile_id() == tab.profile_id()
|
||||
&& entry.space_id() == tab.space_id()
|
||||
&& entry.url() == tab.url()
|
||||
}) {
|
||||
entry.set_title(title);
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn find_history_match(&self, query: &str) -> Option<UrlText> {
|
||||
let normalized_query = query.trim().to_lowercase();
|
||||
if normalized_query.is_empty() {
|
||||
|
||||
@@ -292,17 +292,43 @@ impl BrowserCore {
|
||||
&mut self,
|
||||
tab_id: &TabId,
|
||||
favicon_key: impl Into<String>,
|
||||
) -> Result<(), CoreError> {
|
||||
) -> Result<bool, CoreError> {
|
||||
let favicon_key = favicon_key.into();
|
||||
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].favicon_key() == Some(favicon_key.as_str()) {
|
||||
return Ok(false);
|
||||
}
|
||||
self.tabs[tab_index].set_favicon_key(favicon_key.clone())?;
|
||||
let tab = self.tabs[tab_index].clone();
|
||||
self.set_history_favicon_key_for_tab(&tab, favicon_key);
|
||||
Ok(())
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
/// Replace `tab_id`'s title with the live page title and mirror
|
||||
/// the new title into the history entry that recorded the visit.
|
||||
/// Returns `Ok(true)` only when the title actually changed —
|
||||
/// callers can use this to suppress redundant re-renders.
|
||||
pub fn set_tab_title(
|
||||
&mut self,
|
||||
tab_id: &TabId,
|
||||
title: impl Into<String>,
|
||||
) -> Result<bool, CoreError> {
|
||||
let title = title.into();
|
||||
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].set_title(title.clone()) {
|
||||
return Ok(false);
|
||||
}
|
||||
let tab = self.tabs[tab_index].clone();
|
||||
self.set_history_title_for_tab(&tab, tab.title().to_string());
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
pub fn clear_tab_favicon_key(&mut self, tab_id: &TabId) -> Result<(), CoreError> {
|
||||
|
||||
@@ -91,6 +91,10 @@ impl HistoryEntry {
|
||||
self.favicon_key = None;
|
||||
}
|
||||
|
||||
pub fn set_title(&mut self, title: impl Into<String>) {
|
||||
self.title = title.into();
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn visited_at(&self) -> SystemTime {
|
||||
self.visited_at
|
||||
|
||||
@@ -293,6 +293,20 @@ impl BrowserTab {
|
||||
self.last_active_at = SystemTime::now();
|
||||
}
|
||||
|
||||
/// Set the tab title. Trims whitespace; an entirely-blank title
|
||||
/// keeps the previous value so a page that emits an empty
|
||||
/// `<title>` mid-load doesn't clobber the URL-derived label that
|
||||
/// the new-tab path installed.
|
||||
pub fn set_title(&mut self, title: impl Into<String>) -> bool {
|
||||
let title = title.into();
|
||||
let trimmed = title.trim();
|
||||
if trimmed.is_empty() || self.title == trimmed {
|
||||
return false;
|
||||
}
|
||||
self.title = trimmed.to_string();
|
||||
true
|
||||
}
|
||||
|
||||
/// Navigate in place and record the previous URL in this tab's
|
||||
/// back stack.
|
||||
pub fn navigate_to(&mut self, url: UrlText) {
|
||||
|
||||
@@ -71,6 +71,20 @@ impl UrlText {
|
||||
|
||||
url.host_str().map(str::to_string).unwrap_or_else(|| self.value.clone())
|
||||
}
|
||||
|
||||
/// Resolve the canonical `/favicon.ico` URL for an HTTP(S) page.
|
||||
/// Returns `None` for non-web schemes (`ely://`, `file://`, etc.)
|
||||
/// or URLs missing an authority — those tabs render the URL-derived
|
||||
/// glyph instead of a fetched icon.
|
||||
#[must_use]
|
||||
pub fn favicon_url(&self) -> Option<String> {
|
||||
let url = Url::parse(&self.value).ok()?;
|
||||
if !matches!(url.scheme(), "http" | "https") {
|
||||
return None;
|
||||
}
|
||||
url.host_str()?;
|
||||
url.join("/favicon.ico").ok().map(|favicon| favicon.to_string())
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for UrlText {
|
||||
|
||||
Reference in New Issue
Block a user