From 1a7663cb48d1ba6740a50c3a3a23b89f27e7a5f9 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 31 Oct 2026 13:41:56 +0100 Subject: [PATCH] fix(ui): select the bookmark title when the rename editor opens (#5529) The inline rename field asked for focus without setting a selection, so the caret landed at the end of the default text and typing appended to it. The field now selects the whole title on the frame it takes focus, so typing replaces the old title at once, for both new bookmarks and rename. Later frames leave the user's own caret and selection alone. Tests: three unit tests on rename_field (first focus, multibyte title, caret kept across frames) and two kittest tests driving the panel like a user (typing replaces the title; the caret survives a collapse). Co-Authored-By: Zoo --- crates/ui-egui/src/panels/bookmarks.rs | 87 ++++++++++++++++++++++++-- crates/ui-egui/tests/it/bookmarks.rs | 34 ++++++++++ parity/acrobat-features.toml | 4 +- 3 files changed, 118 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/panels/bookmarks.rs b/crates/ui-egui/src/panels/bookmarks.rs index ccc45d4eb0..2a92a4c227 100644 --- a/crates/ui-egui/src/panels/bookmarks.rs +++ b/crates/ui-egui/src/panels/bookmarks.rs @@ -88,6 +88,24 @@ pub(crate) fn ellipsized_prefix(text: &str, mut fits: impl FnMut(&str) -> bool) } } +/// The inline rename field (#5529). On the frame it takes focus it selects the whole title, so +/// typing replaces the old text at once; later frames leave the user's own caret and selection +/// alone. Same pattern as the zoom box in `chrome.rs`. +fn rename_field(ui: &mut egui::Ui, text: &mut String, id: egui::Id) -> egui::Response { + let first_frame = !ui.memory(|m| m.has_focus(id)); + let resp = ui.add(crate::widgets::line(text).desired_width(ui.available_width() - 8.0).id(id)); + if !resp.has_focus() && !resp.lost_focus() { + if first_frame { + let mut state = egui::TextEdit::load_state(ui.ctx(), id).unwrap_or_default(); + let all = egui::text::CCursorRange::two(egui::text::CCursor::new(0), egui::text::CCursor::new(text.chars().count())); + state.cursor.set_char_range(Some(all)); + state.store(ui.ctx(), id); + } + resp.request_focus(); + } + resp +} + /// How a bookmark's title is drawn: in its own style and colour (ISO 32000-2 §12.3.3, Tables 151 /// and 152), bold in the heavier face and italic slanted, its colour kept readable on the panel. /// Top-level bookmarks without a style keep the medium face. @@ -138,11 +156,7 @@ pub(super) fn outline_item( ui.horizontal(|ui| { ui.add_space(x_text); crate::widgets::begin_dialog(ui); - let edit = crate::widgets::line(text).desired_width(ui.available_width() - 8.0).id(id.with("rename")); - let resp = ui.add(edit); - if !resp.has_focus() && !resp.lost_focus() { - resp.request_focus(); - } + let resp = rename_field(ui, text, id.with("rename")); if ui.input(|i| i.key_pressed(egui::Key::Escape)) { cancel = true; } else if resp.lost_focus() { @@ -360,3 +374,66 @@ mod bookmark_filter { assert!(outline_matches(&outline, &fold("izmir")).is_empty()); } } + +#[cfg(test)] +mod rename_field { + use super::rename_field as field; + + fn cursor_range(ctx: &egui::Context, id: egui::Id) -> Option<(usize, usize)> { + egui::TextEdit::load_state(ctx, id) + .and_then(|s| s.cursor.char_range()) + .map(|r| (r.primary.index.0.min(r.secondary.index.0), r.primary.index.0.max(r.secondary.index.0))) + } + + /// #5529: the rename field opens with the whole title selected, so typing replaces it. + #[test] + fn first_focus_selects_the_whole_title() { + let ctx = egui::Context::default(); + let id = egui::Id::unique("bm-rename-test"); + let mut text = String::from("Untitled"); + for _ in 0..2 { + let output = ctx.run_ui(egui::RawInput::default(), |ui| { + field(ui, &mut text, id); + }); + output.drop_without_applying_deltas(); + } + assert_eq!(cursor_range(&ctx, id), Some((0, 8))); + assert!(ctx.memory(|m| m.has_focus(id)), "the field takes focus"); + } + + /// A Unicode title is counted in characters, not bytes. + #[test] + fn a_multibyte_title_is_selected_in_characters() { + let ctx = egui::Context::default(); + let id = egui::Id::unique("bm-rename-unicode"); + let mut text = String::from("図面（一階平面図）"); + let output = ctx.run_ui(egui::RawInput::default(), |ui| { + field(ui, &mut text, id); + }); + output.drop_without_applying_deltas(); + assert_eq!(cursor_range(&ctx, id), Some((0, 9))); + } + + /// A later frame must not reset the selection the user made; a caret stays a caret. + #[test] + fn a_later_frame_keeps_the_users_selection() { + let ctx = egui::Context::default(); + let id = egui::Id::unique("bm-rename-keep"); + let mut text = String::from("Chapter"); + for _ in 0..2 { + let output = ctx.run_ui(egui::RawInput::default(), |ui| { + field(ui, &mut text, id); + }); + output.drop_without_applying_deltas(); + } + // The user moves the caret to 3 with no selection. + let mut state = egui::TextEdit::load_state(&ctx, id).unwrap_or_default(); + state.cursor.set_char_range(Some(egui::text::CCursorRange::one(egui::text::CCursor::new(3)))); + state.store(&ctx, id); + let output = ctx.run_ui(egui::RawInput::default(), |ui| { + field(ui, &mut text, id); + }); + output.drop_without_applying_deltas(); + assert_eq!(cursor_range(&ctx, id), Some((3, 3)), "the user's caret is kept"); + } +} diff --git a/crates/ui-egui/tests/it/bookmarks.rs b/crates/ui-egui/tests/it/bookmarks.rs index 1f07102a18..d5cd5efd2f 100644 --- a/crates/ui-egui/tests/it/bookmarks.rs +++ b/crates/ui-egui/tests/it/bookmarks.rs @@ -119,6 +119,40 @@ fn the_panel_keeps_its_width_while_the_pointer_moves() { assert_eq!(panel_width(&h), before, "the Bookmarks panel drifted wider"); } +/// #5529: the title is selected when the rename editor opens, so typing replaces it at once, +/// with no Ctrl+A. New bookmarks and double-click rename both get this. +#[test] +fn typing_replaces_the_title_when_the_editor_opens() { + let mut h = harness(); + h.key_press_modifiers(Modifiers::COMMAND, Key::B); + h.run_steps(3); + field(&h, "Untitled").type_text("Intro"); // the whole default is replaced + h.key_press(Key::Enter); + h.run_steps(3); + assert_eq!(outline(&h), [("Intro".into(), Some(0), 0)]); + + menu(&mut h, "Intro", "Rename"); + field(&h, "Intro").type_text("Introduction"); + h.key_press(Key::Enter); + h.run_steps(3); + assert_eq!(outline(&h)[0].0, "Introduction"); +} + +/// The selection is one-shot: after the user collapses it to a caret, later frames must not +/// reselect, so typing at that caret inserts. +#[test] +fn a_rename_editor_keeps_the_users_caret_across_frames() { + let mut h = harness(); + h.key_press_modifiers(Modifiers::COMMAND, Key::B); + h.run_steps(3); + h.key_press(Key::ArrowLeft); // collapses the opening selection to the caret at 0 + h.run_steps(2); + field(&h, "Untitled").type_text("I"); // inserts at the caret, does not replace + h.key_press(Key::Enter); + h.run_steps(3); + assert_eq!(outline(&h)[0].0, "IUntitled"); +} + #[test] fn escape_cancels_a_rename() { let mut h = harness(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 0b6f658dd6..d0377fc883 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -3291,8 +3291,8 @@ milestone = "M4" status = "shipped" commands = [] tools = ["bookmark_rename"] -evidence = ["crates/ui-egui/tests/it/bookmarks.rs::new_rename_reorder_indent_delete_and_undo", "crates/ui-egui/tests/it/bookmarks.rs::escape_cancels_a_rename", "crates/organize/src/tests.rs::bookmarks_add_rename_move_delete", "crates/organize/src/tests.rs::bookmark_edits_keep_unknown_keys_and_touch_few_objects"] -notes = "" +evidence = ["crates/ui-egui/tests/it/bookmarks.rs::new_rename_reorder_indent_delete_and_undo", "crates/ui-egui/tests/it/bookmarks.rs::escape_cancels_a_rename", "crates/ui-egui/tests/it/bookmarks.rs::typing_replaces_the_title_when_the_editor_opens", "crates/ui-egui/tests/it/bookmarks.rs::a_rename_editor_keeps_the_users_caret_across_frames", "crates/organize/src/tests.rs::bookmarks_add_rename_move_delete", "crates/organize/src/tests.rs::bookmark_edits_keep_unknown_keys_and_touch_few_objects"] +notes = "The inline editor opens with the whole title selected, so typing replaces it at once (#5529); a caret the user moves afterwards is kept by later frames." [[feature]] id = "organize.bookmark-delete"