From 22b1abb36dcd159249906e3c264c5c329a430244 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Sun, 26 Dec 2021 12:43:22 -0800 Subject: [PATCH 1/4] testsuite: ignore texthistory selection on delete/backspace We don't need to apply these here, as it will clear the selection which is needed for the undo. Otherwise we won't be able to test that we end up at the right selection afterwards. --- testsuite/gtk/texthistory.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/testsuite/gtk/texthistory.c b/testsuite/gtk/texthistory.c index a740d3523b..4da0dc5ab0 100644 --- a/testsuite/gtk/texthistory.c +++ b/testsuite/gtk/texthistory.c @@ -251,8 +251,6 @@ run_test (const Command *commands, set_selection (text, cmd->location, cmd->end_location); else if (strlen (cmd->text) == 1) set_selection (text, cmd->location, -1); - else - set_selection (text, -1, -1); command_delete_key (cmd, text); break; @@ -261,8 +259,6 @@ run_test (const Command *commands, set_selection (text, cmd->location, cmd->end_location); else if (strlen (cmd->text) == 1) set_selection (text, cmd->end_location, -1); - else - set_selection (text, -1, -1); command_delete_key (cmd, text); break; From 99d8dd751e74bd383c7c0cc775cc6cd3ff2080ea Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Sun, 26 Dec 2021 12:46:16 -0800 Subject: [PATCH 2/4] testsuite: add failing test for delete selection This adds a test to expose the failure of #4575 which results in the selection being incorrect when performing a delete as we are likely already in a begin_user_action()/end_user_action() pair. Related #4575 --- testsuite/gtk/texthistory.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/testsuite/gtk/texthistory.c b/testsuite/gtk/texthistory.c index 4da0dc5ab0..933c8611e9 100644 --- a/testsuite/gtk/texthistory.c +++ b/testsuite/gtk/texthistory.c @@ -619,6 +619,22 @@ test_issue_4276 (void) run_test (commands, G_N_ELEMENTS (commands), 0); } +static void +test_issue_4575 (void) +{ + const Command commands[] = { + { INSERT, 0, -1, "this is some text", "this is some text", SET, UNSET, UNSET }, + { SELECT, 5, 8, NULL, NULL, SET, UNSET, UNSET }, + { BEGIN_USER, -1, -1, NULL, NULL, UNSET, UNSET, UNSET }, + { DELETE_KEY, 5, 8, "is ", "this some text", UNSET, UNSET, UNSET, IGNORE_SELECT }, + { END_USER, -1, -1, NULL, NULL, SET, UNSET, UNSET }, + { UNDO, -1, -1, NULL, "this is some text", SET, SET, UNSET }, + { CHECK_SELECT, 5, 8, NULL, "this is some text", SET, SET, UNSET }, + }; + + run_test (commands, G_N_ELEMENTS (commands), 0); +} + int main (int argc, char *argv[]) @@ -640,6 +656,7 @@ main (int argc, g_test_add_func ("/Gtk/TextHistory/test13", test13); g_test_add_func ("/Gtk/TextHistory/test14", test14); g_test_add_func ("/Gtk/TextHistory/issue_4276", test_issue_4276); + g_test_add_func ("/Gtk/TextHistory/issue_4575", test_issue_4575); return g_test_run (); } From e7871fbc433fad592e227ead834346aa3db8a235 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Sun, 26 Dec 2021 12:47:45 -0800 Subject: [PATCH 3/4] texthistory: always track selection bounds It's cheap to store the selection position, so always set it even if we are in a user section. Otherwise, we risk not having the right position when starting a delete action within a begin_user_action(), end_user_action() pair. Related #4575 --- gtk/gtktexthistory.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/gtk/gtktexthistory.c b/gtk/gtktexthistory.c index 3b9379ccc3..5f3f2cb56c 100644 --- a/gtk/gtktexthistory.c +++ b/gtk/gtktexthistory.c @@ -981,11 +981,8 @@ gtk_text_history_selection_changed (GtkTextHistory *self, return_if_applying (self); return_if_irreversible (self); - if (self->in_user == 0 && self->irreversible == 0) - { - self->selection.insert = CLAMP (selection_insert, -1, G_MAXINT); - self->selection.bound = CLAMP (selection_bound, -1, G_MAXINT); - } + self->selection.insert = CLAMP (selection_insert, -1, G_MAXINT); + self->selection.bound = CLAMP (selection_bound, -1, G_MAXINT); } void From 344ad0355efa35f80b3c4a597e91a5fe3f438eed Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Sun, 26 Dec 2021 12:56:42 -0800 Subject: [PATCH 4/4] textview: scroll insert onscreen after undo/redo After performing an action such as undo/redo, we need to actually scroll to the position where the operation occurred. I do note that the scroll here seems to often get invalidated if it is pages away, and we never make the full scroll. But I've seen this all over the place elsewhere too and that needs to be handled, most likely, as a more comprehensive fix for scrolling during line validation. Related #4575 --- gtk/gtktextview.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/gtk/gtktextview.c b/gtk/gtktextview.c index f8e1f0d1ed..00b361f8a2 100644 --- a/gtk/gtktextview.c +++ b/gtk/gtktextview.c @@ -10101,7 +10101,11 @@ gtk_text_view_real_undo (GtkWidget *widget, GtkTextView *text_view = GTK_TEXT_VIEW (widget); if (gtk_text_view_get_editable (text_view)) - gtk_text_buffer_undo (text_view->priv->buffer); + { + gtk_text_buffer_undo (text_view->priv->buffer); + gtk_text_view_scroll_mark_onscreen (text_view, + gtk_text_buffer_get_insert (text_view->priv->buffer)); + } } static void @@ -10112,7 +10116,11 @@ gtk_text_view_real_redo (GtkWidget *widget, GtkTextView *text_view = GTK_TEXT_VIEW (widget); if (gtk_text_view_get_editable (text_view)) - gtk_text_buffer_redo (text_view->priv->buffer); + { + gtk_text_buffer_redo (text_view->priv->buffer); + gtk_text_view_scroll_mark_onscreen (text_view, + gtk_text_buffer_get_insert (text_view->priv->buffer)); + } } static void