fix(window): clamp restored X11 window position, sanity-guard absurd coords (A2)
initial_window_position fed saved window_x/window_y straight into Position::Specific with no bounds check, so a saved position on a since- disconnected monitor (or after a resolution shrink) could open the window fully off-screen on a bare X11 WM that doesn't clamp. New pure clamp_window_position seam: given display bounds it pulls a partly-offscreen window back inside, centers one parked on a vanished monitor, and crucially PRESERVES legitimate multi-monitor negative-origin coordinates (a naive clamp-to-0 would break that). iced 0.14 has no dependency-free way to learn the virtual-desktop bounds before the window exists, so screen_bounds() returns None for now and the clamp applies a sanity envelope (reject |coord| > 32000 -> Centered) while preserving today's restore behavior; the full clamp is unit-tested and ready for when bounds can be supplied. Five clamp tests (inside, edge-clamp, disconnected, negative-origin, None-sanity) + existing tests updated. No new deps, no wire change. Codex-implemented (gpt-5.5 xhigh), senior-reviewed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+184
-11
@@ -1067,7 +1067,13 @@ pub fn run_gui() -> iced::Result {
|
||||
// position, so we center there and leave placement to the compositor.
|
||||
let saved = AppConfig::load();
|
||||
let init_size = iced::Size::new(saved.window_width, saved.window_height);
|
||||
let init_position = initial_window_position(saved.window_x, saved.window_y, is_wayland());
|
||||
let init_position = initial_window_position(
|
||||
saved.window_x,
|
||||
saved.window_y,
|
||||
saved.window_width.round() as i32,
|
||||
saved.window_height.round() as i32,
|
||||
is_wayland(),
|
||||
);
|
||||
iced::application(AppState::default, update, view_with_background)
|
||||
.title("PeerSpeak P2P Voice Chat")
|
||||
.theme(theme)
|
||||
@@ -1130,16 +1136,96 @@ fn is_wayland() -> bool {
|
||||
fn initial_window_position(
|
||||
saved_x: Option<i32>,
|
||||
saved_y: Option<i32>,
|
||||
win_w: i32,
|
||||
win_h: i32,
|
||||
is_wayland: bool,
|
||||
) -> iced::window::Position {
|
||||
match (saved_x, saved_y) {
|
||||
(Some(x), Some(y)) if !is_wayland => {
|
||||
iced::window::Position::Specific(iced::Point::new(x as f32, y as f32))
|
||||
clamp_window_position(x, y, win_w, win_h, screen_bounds())
|
||||
}
|
||||
_ => iced::window::Position::Centered,
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
struct ScreenBounds {
|
||||
x: i32,
|
||||
y: i32,
|
||||
width: i32,
|
||||
height: i32,
|
||||
}
|
||||
|
||||
const MIN_VISIBLE_WINDOW_MARGIN: i32 = 48;
|
||||
const WINDOW_POSITION_SANITY_LIMIT: i64 = 32_000;
|
||||
|
||||
fn screen_bounds() -> Option<ScreenBounds> {
|
||||
// iced 0.14 only exposes monitor size through window tasks after a window
|
||||
// exists, not the startup virtual desktop bounds needed here. Keep the pure
|
||||
// clamp ready for when dependency-free bounds can be supplied.
|
||||
None
|
||||
}
|
||||
|
||||
fn clamp_window_position(
|
||||
saved_x: i32,
|
||||
saved_y: i32,
|
||||
win_w: i32,
|
||||
win_h: i32,
|
||||
bounds: Option<ScreenBounds>,
|
||||
) -> iced::window::Position {
|
||||
let specific = |x, y| iced::window::Position::Specific(iced::Point::new(x as f32, y as f32));
|
||||
let win_w = win_w.max(1);
|
||||
let win_h = win_h.max(1);
|
||||
|
||||
if let Some(bounds) = bounds {
|
||||
if bounds.width <= 0
|
||||
|| bounds.height <= 0
|
||||
|| !has_min_visible_overlap(saved_x, win_w, bounds.x, bounds.width)
|
||||
|| !has_min_visible_overlap(saved_y, win_h, bounds.y, bounds.height)
|
||||
{
|
||||
return iced::window::Position::Centered;
|
||||
}
|
||||
|
||||
return specific(
|
||||
clamp_window_position_axis(saved_x, win_w, bounds.x, bounds.width),
|
||||
clamp_window_position_axis(saved_y, win_h, bounds.y, bounds.height),
|
||||
);
|
||||
}
|
||||
|
||||
if i64::from(saved_x).abs() > WINDOW_POSITION_SANITY_LIMIT
|
||||
|| i64::from(saved_y).abs() > WINDOW_POSITION_SANITY_LIMIT
|
||||
{
|
||||
return iced::window::Position::Centered;
|
||||
}
|
||||
|
||||
specific(saved_x, saved_y)
|
||||
}
|
||||
|
||||
fn has_min_visible_overlap(start: i32, len: i32, bounds_start: i32, bounds_len: i32) -> bool {
|
||||
let required = MIN_VISIBLE_WINDOW_MARGIN
|
||||
.min(len)
|
||||
.min(bounds_len)
|
||||
.max(1);
|
||||
let start = i64::from(start);
|
||||
let end = start + i64::from(len);
|
||||
let bounds_start = i64::from(bounds_start);
|
||||
let bounds_end = bounds_start + i64::from(bounds_len);
|
||||
let overlap = end.min(bounds_end) - start.max(bounds_start);
|
||||
|
||||
overlap >= i64::from(required)
|
||||
}
|
||||
|
||||
fn clamp_window_position_axis(start: i32, len: i32, bounds_start: i32, bounds_len: i32) -> i32 {
|
||||
if len >= bounds_len {
|
||||
return bounds_start;
|
||||
}
|
||||
|
||||
let bounds_start = i64::from(bounds_start);
|
||||
let max_start = bounds_start + i64::from(bounds_len - len);
|
||||
|
||||
i64::from(start).clamp(bounds_start, max_start) as i32
|
||||
}
|
||||
|
||||
fn subscription(state: &AppState) -> Subscription<AppMessage> {
|
||||
let core_sub = Subscription::run(core_subscription).map(AppMessage::UiEventReceived);
|
||||
let event_sub = iced::event::listen().map(AppMessage::EventOccurred);
|
||||
@@ -7775,10 +7861,11 @@ impl Program<AppMessage> for Icon {
|
||||
mod tests {
|
||||
use super::{
|
||||
attachment_default_name, clear_expired_clock_skew_warning, format_clock_skew_duration,
|
||||
format_duration, format_relative_ago, initial_window_position, now_playing_label,
|
||||
reconnect_attempt_chime, reconnected_chime, set_peer_gate_config, set_peer_volume_config, show_clock_skew_warning, update, AppConfig,
|
||||
AppMessage, AppState, AttachmentCache, AttachmentState, ChatEntry, ClockSkewBanner,
|
||||
GateMeter, METER_MAX, UiEvent, CLOCK_SKEW_WARNING_VISIBLE_SECS,
|
||||
clamp_window_position, format_duration, format_relative_ago, initial_window_position,
|
||||
now_playing_label, reconnect_attempt_chime, reconnected_chime, set_peer_gate_config,
|
||||
set_peer_volume_config, show_clock_skew_warning, update, AppConfig, AppMessage, AppState,
|
||||
AttachmentCache, AttachmentState, ChatEntry, ClockSkewBanner, GateMeter, ScreenBounds,
|
||||
METER_MAX, UiEvent, CLOCK_SKEW_WARNING_VISIBLE_SECS,
|
||||
};
|
||||
use iroh::SecretKey;
|
||||
|
||||
@@ -8274,10 +8361,96 @@ mod tests {
|
||||
assert!(!config.peer_volume.contains_key(&id.to_string()));
|
||||
}
|
||||
|
||||
fn assert_specific_position(position: iced::window::Position, x: i32, y: i32) {
|
||||
match position {
|
||||
iced::window::Position::Specific(p) => {
|
||||
assert_eq!(p.x, x as f32);
|
||||
assert_eq!(p.y, y as f32);
|
||||
}
|
||||
other => panic!("expected Specific, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clamp_window_position_preserves_fully_inside_bounds() {
|
||||
let bounds = ScreenBounds {
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 1200,
|
||||
height: 900,
|
||||
};
|
||||
|
||||
assert_specific_position(
|
||||
clamp_window_position(120, 80, 900, 760, Some(bounds)),
|
||||
120,
|
||||
80,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clamp_window_position_pulls_visible_edge_back_inside_bounds() {
|
||||
let bounds = ScreenBounds {
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 1000,
|
||||
height: 800,
|
||||
};
|
||||
|
||||
assert_specific_position(
|
||||
clamp_window_position(760, 650, 300, 200, Some(bounds)),
|
||||
700,
|
||||
600,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clamp_window_position_centers_disconnected_monitor_position() {
|
||||
let bounds = ScreenBounds {
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 1000,
|
||||
height: 800,
|
||||
};
|
||||
|
||||
assert!(matches!(
|
||||
clamp_window_position(5000, 5000, 900, 760, Some(bounds)),
|
||||
iced::window::Position::Centered
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clamp_window_position_preserves_negative_origin_monitor_position() {
|
||||
let bounds = ScreenBounds {
|
||||
x: -1920,
|
||||
y: -200,
|
||||
width: 1920,
|
||||
height: 1080,
|
||||
};
|
||||
|
||||
assert_specific_position(
|
||||
clamp_window_position(-1800, -120, 900, 760, Some(bounds)),
|
||||
-1800,
|
||||
-120,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clamp_window_position_without_bounds_preserves_sane_and_rejects_absurd() {
|
||||
assert_specific_position(clamp_window_position(120, -40, 900, 760, None), 120, -40);
|
||||
assert!(matches!(
|
||||
clamp_window_position(32_001, 0, 900, 760, None),
|
||||
iced::window::Position::Centered
|
||||
));
|
||||
assert!(matches!(
|
||||
clamp_window_position(0, -32_001, 900, 760, None),
|
||||
iced::window::Position::Centered
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn x11_restores_saved_window_position() {
|
||||
// On X11 (is_wayland = false) a saved position becomes Specific(x, y).
|
||||
match initial_window_position(Some(120), Some(-40), false) {
|
||||
match initial_window_position(Some(120), Some(-40), 900, 760, false) {
|
||||
iced::window::Position::Specific(p) => {
|
||||
assert_eq!(p.x, 120.0);
|
||||
assert_eq!(p.y, -40.0); // negative coords (left/above primary) preserved
|
||||
@@ -8289,7 +8462,7 @@ mod tests {
|
||||
#[test]
|
||||
fn wayland_always_centers_even_with_saved_position() {
|
||||
assert!(matches!(
|
||||
initial_window_position(Some(120), Some(40), true),
|
||||
initial_window_position(Some(120), Some(40), 900, 760, true),
|
||||
iced::window::Position::Centered
|
||||
));
|
||||
}
|
||||
@@ -8297,16 +8470,16 @@ mod tests {
|
||||
#[test]
|
||||
fn missing_or_partial_saved_position_centers() {
|
||||
assert!(matches!(
|
||||
initial_window_position(None, None, false),
|
||||
initial_window_position(None, None, 900, 760, false),
|
||||
iced::window::Position::Centered
|
||||
));
|
||||
// A half-saved position (one axis missing) is not enough to restore.
|
||||
assert!(matches!(
|
||||
initial_window_position(Some(10), None, false),
|
||||
initial_window_position(Some(10), None, 900, 760, false),
|
||||
iced::window::Position::Centered
|
||||
));
|
||||
assert!(matches!(
|
||||
initial_window_position(None, Some(10), false),
|
||||
initial_window_position(None, Some(10), 900, 760, false),
|
||||
iced::window::Position::Centered
|
||||
));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user