widget: Deprecate internal signals

The text-{modified,inserted,deleted,scrolled} signals are already
documented to be internal and possibly not emitted. Deprecate them
and document them as never emitted; and internally just call the
acccessible object directly instead of via the extraneous signal
emission.

Fixes: https://gitlab.gnome.org/GNOME/vte/-/issues/332

8 files changed, 162 insertions, 181 deletions

diff --git a/src/vte.cc b/src/vte.cc
index d9a2ec3d..534b722c 100644
--- a/src/vte.cc
+++ b/src/vte.cc
@@ -268,6 +268,10 @@ vte_g_array_fill(GArray *array, gconstpointer item, guint final_size)
 void
 Terminal::unset_widget() noexcept
 {
+#ifdef WITH_A11Y
+        set_accessible(nullptr);
+#endif
+
         m_real_widget = nullptr;
         m_terminal = nullptr;
         m_widget = nullptr;
@@ -1024,63 +1028,6 @@ Terminal::emit_decrease_font_size()
 	g_signal_emit(m_terminal, signals[SIGNAL_DECREASE_FONT_SIZE], 0);
 }
 
-/* Emit a "text-inserted" signal. */
-void
-Terminal::emit_text_inserted()
-{
-#ifdef WITH_A11Y
-	if (!m_accessible_emit) {
-		return;
-	}
-	_vte_debug_print(VTE_DEBUG_SIGNALS,
-			"Emitting `text-inserted'.\n");
-	g_signal_emit(m_terminal, signals[SIGNAL_TEXT_INSERTED], 0);
-#endif
-}
-
-/* Emit a "text-deleted" signal. */
-void
-Terminal::emit_text_deleted()
-{
-#ifdef WITH_A11Y
-	if (!m_accessible_emit) {
-		return;
-	}
-	_vte_debug_print(VTE_DEBUG_SIGNALS,
-			"Emitting `text-deleted'.\n");
-	g_signal_emit(m_terminal, signals[SIGNAL_TEXT_DELETED], 0);
-#endif
-}
-
-/* Emit a "text-modified" signal. */
-void
-Terminal::emit_text_modified()
-{
-#ifdef WITH_A11Y
-	if (!m_accessible_emit) {
-		return;
-	}
-	_vte_debug_print(VTE_DEBUG_SIGNALS,
-                         "Emitting `text-modified'.\n");
-	g_signal_emit(m_terminal, signals[SIGNAL_TEXT_MODIFIED], 0);
-#endif
-}
-
-/* Emit a "text-scrolled" signal. */
-void
-Terminal::emit_text_scrolled(long delta)
-{
-#ifdef WITH_A11Y
-	if (!m_accessible_emit) {
-		return;
-	}
-	_vte_debug_print(VTE_DEBUG_SIGNALS,
-			"Emitting `text-scrolled'(%ld).\n", delta);
-        // FIXMEchpe fix signal signature?
-	g_signal_emit(m_terminal, signals[SIGNAL_TEXT_SCROLLED], 0, (int)delta);
-#endif
-}
-
 void
 Terminal::emit_copy_clipboard()
 {
@@ -10231,16 +10178,6 @@ Terminal::terminate_child() noexcept
         return true;
 }
 
-/* We need this bit of glue to ensure that accessible objects will always
- * get signals. */
-void
-Terminal::subscribe_accessible_events()
-{
-#ifdef WITH_A11Y
-	m_accessible_emit = true;
-#endif
-}
-
 void
 Terminal::select_text(vte::grid::column_t start_col,
                                 vte::grid::row_t start_row,
diff --git a/src/vte/vteterminal.h b/src/vte/vteterminal.h
index 736b3e77..346523ad 100644
--- a/src/vte/vteterminal.h
+++ b/src/vte/vteterminal.h
@@ -95,10 +95,15 @@ struct _VteTerminalClass {
 	void (*increase_font_size)(VteTerminal* terminal);
 	void (*decrease_font_size)(VteTerminal* terminal);
 
+#if _VTE_GTK == 3
+	/*< private >*/
 	void (*text_modified)(VteTerminal* terminal);
 	void (*text_inserted)(VteTerminal* terminal);
 	void (*text_deleted)(VteTerminal* terminal);
 	void (*text_scrolled)(VteTerminal* terminal, gint delta);
+#endif /* _VTE_GTK == 3 */
+
+	/*< protected >*/
 	void (*copy_clipboard)(VteTerminal* terminal);
 	void (*paste_clipboard)(VteTerminal* terminal);
 
diff --git a/src/vteaccess.cc b/src/vteaccess.cc
index 6a7e91d5..87633a06 100644
--- a/src/vteaccess.cc
+++ b/src/vteaccess.cc
@@ -378,11 +378,21 @@ vte_terminal_accessible_maybe_emit_text_caret_moved(VteTerminalAccessible *acces
         }
 }
 
-/* A signal handler to catch "text-inserted/deleted/modified" signals. */
-static void
-vte_terminal_accessible_text_modified(VteTerminal *terminal, gpointer data)
+void
+_vte_terminal_accessible_text_inserted(VteTerminalAccessible* accessible)
+{
+        _vte_terminal_accessible_text_modified(accessible);
+}
+
+void
+_vte_terminal_accessible_text_deleted (VteTerminalAccessible* accessible)
+{
+        _vte_terminal_accessible_text_modified(accessible);
+}
+
+void
+_vte_terminal_accessible_text_modified(VteTerminalAccessible* accessible)
 {
-        VteTerminalAccessible *accessible = (VteTerminalAccessible *)data;
 	VteTerminalAccessiblePrivate *priv = (VteTerminalAccessiblePrivate *)_vte_terminal_accessible_get_instance_private(accessible);
         GString *old_text;
         GArray *old_characters;
@@ -491,13 +501,10 @@ vte_terminal_accessible_text_modified(VteTerminal *terminal, gpointer data)
         g_array_free(old_characters, TRUE);
 }
 
-/* A signal handler to catch "text-scrolled" signals. */
-static void
-vte_terminal_accessible_text_scrolled(VteTerminal *terminal,
-				      gint howmuch,
-				      gpointer data)
+void
+_vte_terminal_accessible_text_scrolled(VteTerminalAccessible* accessible,
+                                       long howmuch)
 {
-        VteTerminalAccessible *accessible = (VteTerminalAccessible *)data;
 	VteTerminalAccessiblePrivate *priv = (VteTerminalAccessiblePrivate *)_vte_terminal_accessible_get_instance_private(accessible);
 	struct _VteCharAttributes attr;
 	long delta, row_count;
@@ -507,6 +514,9 @@ vte_terminal_accessible_text_scrolled(VteTerminal *terminal,
         /* g_assert(howmuch != 0); */
         if (howmuch == 0) return;
 
+        auto widget = gtk_accessible_get_widget(GTK_ACCESSIBLE(accessible));
+        auto terminal = VTE_TERMINAL(widget);
+
         row_count = vte_terminal_get_row_count(terminal);
 	if (((howmuch < 0) && (howmuch <= -row_count)) ||
 	    ((howmuch > 0) && (howmuch >= row_count))) {
@@ -707,31 +717,21 @@ vte_terminal_accessible_selection_changed (VteTerminal *terminal,
 }
 
 static void
-vte_terminal_accessible_initialize (AtkObject *obj, gpointer data)
+vte_terminal_accessible_initialize(AtkObject *obj,
+                                   gpointer data)
 {
-	VteTerminal *terminal = VTE_TERMINAL (data);
+        auto accessible = VTE_TERMINAL_ACCESSIBLE(obj);
+        VteTerminal *terminal = VTE_TERMINAL(data);
         const char *window_title;
 
 	ATK_OBJECT_CLASS (_vte_terminal_accessible_parent_class)->initialize (obj, data);
 
         auto impl = IMPL(terminal);
         try {
-                impl->subscribe_accessible_events();
+                impl->set_accessible(accessible);
         } catch (...) {
         }
 
-	g_signal_connect(terminal, "text-inserted",
-			 G_CALLBACK(vte_terminal_accessible_text_modified),
-			 obj);
-	g_signal_connect(terminal, "text-deleted",
-			 G_CALLBACK(vte_terminal_accessible_text_modified),
-			 obj);
-	g_signal_connect(terminal, "text-modified",
-			 G_CALLBACK(vte_terminal_accessible_text_modified),
-			 obj);
-	g_signal_connect(terminal, "text-scrolled",
-			 G_CALLBACK(vte_terminal_accessible_text_scrolled),
-			 obj);
 	g_signal_connect(terminal, "cursor-moved",
 			 G_CALLBACK(vte_terminal_accessible_invalidate_cursor),
 			 obj);
@@ -790,16 +790,6 @@ vte_terminal_accessible_finalize(GObject *object)
 		g_signal_handlers_disconnect_matched(widget,
 						     (GSignalMatchType)(G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA),
 						     0, 0, NULL,
-						     (void *)vte_terminal_accessible_text_modified,
-						     object);
-		g_signal_handlers_disconnect_matched(widget,
-						     (GSignalMatchType)(G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA),
-						     0, 0, NULL,
-						     (void *)vte_terminal_accessible_text_scrolled,
-						     object);
-		g_signal_handlers_disconnect_matched(widget,
-						     (GSignalMatchType)(G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA),
-						     0, 0, NULL,
 						     (void *)vte_terminal_accessible_invalidate_cursor,
 						     object);
 		g_signal_handlers_disconnect_matched(widget,
@@ -812,6 +802,13 @@ vte_terminal_accessible_finalize(GObject *object)
 						     0, 0, NULL,
 						     (void *)vte_terminal_accessible_visibility_notify,
 						     object);
+
+                auto terminal = VTE_TERMINAL(widget);
+                auto impl = IMPL(terminal);
+                try {
+                        impl->set_accessible(nullptr);
+                } catch (...) {
+                }
 	}
 
 	if (priv->snapshot_text != NULL) {
diff --git a/src/vteaccess.h b/src/vteaccess.h
index 31f21601..1ec9bd7c 100644
--- a/src/vteaccess.h
+++ b/src/vteaccess.h
@@ -48,4 +48,13 @@ struct _VteTerminalAccessibleClass {
 
 GType _vte_terminal_accessible_get_type(void);
 
+void _vte_terminal_accessible_text_modified(VteTerminalAccessible* accessible);
+
+void _vte_terminal_accessible_text_inserted(VteTerminalAccessible* accessible);
+
+void _vte_terminal_accessible_text_deleted (VteTerminalAccessible* accessible);
+
+void _vte_terminal_accessible_text_scrolled(VteTerminalAccessible* accessible,
+                                            long delta);
+
 G_END_DECLS
diff --git a/src/vtegtk.cc b/src/vtegtk.cc
index 24623405..86f0f682 100644
--- a/src/vtegtk.cc
+++ b/src/vtegtk.cc
@@ -1255,10 +1255,12 @@ vte_terminal_class_init(VteTerminalClass *klass)
 	klass->increase_font_size = NULL;
 	klass->decrease_font_size = NULL;
 
+#if VTE_GTK == 3
 	klass->text_modified = NULL;
 	klass->text_inserted = NULL;
 	klass->text_deleted = NULL;
 	klass->text_scrolled = NULL;
+#endif /* VTE_GTK == 3 */
 
 	klass->copy_clipboard = vte_terminal_real_copy_clipboard;
 	klass->paste_clipboard = vte_terminal_real_paste_clipboard;
@@ -1777,90 +1779,73 @@ vte_terminal_class_init(VteTerminalClass *klass)
                                    G_OBJECT_CLASS_TYPE(klass),
                                    g_cclosure_marshal_VOID__VOIDv);
 
+#if VTE_GTK == 3
+        /* These signals are deprecated and never emitted,
+         * but need to be kept for ABI compatibility on gtk3.
+         */
+
         /**
          * VteTerminal::text-modified:
-         * @vteterminal: the object which received the signal
+         * @vteterminal:
          *
-         * An internal signal used for communication between the terminal and
-         * its accessibility peer. May not be emitted under certain
-         * circumstances.
+         * Deprecated: 0.66: This signal is never emitted.
          */
-        signals[SIGNAL_TEXT_MODIFIED] =
-                g_signal_new(I_("text-modified"),
-                             G_OBJECT_CLASS_TYPE(klass),
-                             G_SIGNAL_RUN_LAST,
-                             G_STRUCT_OFFSET(VteTerminalClass, text_modified),
-                             NULL,
-                             NULL,
-                             g_cclosure_marshal_VOID__VOID,
-                             G_TYPE_NONE, 0);
-        g_signal_set_va_marshaller(signals[SIGNAL_TEXT_MODIFIED],
-                                   G_OBJECT_CLASS_TYPE(klass),
-                                   g_cclosure_marshal_VOID__VOIDv);
+        g_signal_new(I_("text-modified"),
+                     G_OBJECT_CLASS_TYPE(klass),
+                     GSignalFlags(G_SIGNAL_RUN_LAST | G_SIGNAL_DEPRECATED),
+                     G_STRUCT_OFFSET(VteTerminalClass, text_modified),
+                     NULL,
+                     NULL,
+                     g_cclosure_marshal_VOID__VOID,
+                     G_TYPE_NONE, 0);
 
         /**
          * VteTerminal::text-inserted:
-         * @vteterminal: the object which received the signal
+         * @vteterminal:
          *
-         * An internal signal used for communication between the terminal and
-         * its accessibility peer. May not be emitted under certain
-         * circumstances.
+         * Deprecated: 0.66: This signal is never emitted.
          */
-        signals[SIGNAL_TEXT_INSERTED] =
-                g_signal_new(I_("text-inserted"),
-                             G_OBJECT_CLASS_TYPE(klass),
-                             G_SIGNAL_RUN_LAST,
-                             G_STRUCT_OFFSET(VteTerminalClass, text_inserted),
-                             NULL,
-                             NULL,
-                             g_cclosure_marshal_VOID__VOID,
-                             G_TYPE_NONE, 0);
-        g_signal_set_va_marshaller(signals[SIGNAL_TEXT_INSERTED],
-                                   G_OBJECT_CLASS_TYPE(klass),
-                                   g_cclosure_marshal_VOID__VOIDv);
+        g_signal_new(I_("text-inserted"),
+                     G_OBJECT_CLASS_TYPE(klass),
+                     GSignalFlags(G_SIGNAL_RUN_LAST | G_SIGNAL_DEPRECATED),
+                     G_STRUCT_OFFSET(VteTerminalClass, text_inserted),
+                     NULL,
+                     NULL,
+                     g_cclosure_marshal_VOID__VOID,
+                     G_TYPE_NONE, 0);
 
         /**
          * VteTerminal::text-deleted:
-         * @vteterminal: the object which received the signal
+         * @vteterminal:
          *
-         * An internal signal used for communication between the terminal and
-         * its accessibility peer. May not be emitted under certain
-         * circumstances.
+         * Deprecated: 0.66: This signal is never emitted.
          */
-        signals[SIGNAL_TEXT_DELETED] =
-                g_signal_new(I_("text-deleted"),
-                             G_OBJECT_CLASS_TYPE(klass),
-                             G_SIGNAL_RUN_LAST,
-                             G_STRUCT_OFFSET(VteTerminalClass, text_deleted),
-                             NULL,
-                             NULL,
-                             g_cclosure_marshal_VOID__VOID,
-                             G_TYPE_NONE, 0);
-        g_signal_set_va_marshaller(signals[SIGNAL_TEXT_DELETED],
-                                   G_OBJECT_CLASS_TYPE(klass),
-                                   g_cclosure_marshal_VOID__VOIDv);
+        g_signal_new(I_("text-deleted"),
+                     G_OBJECT_CLASS_TYPE(klass),
+                     GSignalFlags(G_SIGNAL_RUN_LAST | G_SIGNAL_DEPRECATED),
+                     G_STRUCT_OFFSET(VteTerminalClass, text_deleted),
+                     NULL,
+                     NULL,
+                     g_cclosure_marshal_VOID__VOID,
+                     G_TYPE_NONE, 0);
 
         /**
          * VteTerminal::text-scrolled:
-         * @vteterminal: the object which received the signal
-         * @delta: the number of lines scrolled
+         * @vteterminal:
+         * @delta:
          *
-         * An internal signal used for communication between the terminal and
-         * its accessibility peer. May not be emitted under certain
-         * circumstances.
+         * Deprecated: 0.66: This signal is never emitted.
          */
-        signals[SIGNAL_TEXT_SCROLLED] =
-                g_signal_new(I_("text-scrolled"),
-                             G_OBJECT_CLASS_TYPE(klass),
-                             G_SIGNAL_RUN_LAST,
-                             G_STRUCT_OFFSET(VteTerminalClass, text_scrolled),
-                             NULL,
-                             NULL,
-                             g_cclosure_marshal_VOID__INT,
-                             G_TYPE_NONE, 1, G_TYPE_INT);
-        g_signal_set_va_marshaller(signals[SIGNAL_TEXT_SCROLLED],
-                                   G_OBJECT_CLASS_TYPE(klass),
-                                   g_cclosure_marshal_VOID__INTv);
+        g_signal_new(I_("text-scrolled"),
+                     G_OBJECT_CLASS_TYPE(klass),
+                     GSignalFlags(G_SIGNAL_RUN_LAST | G_SIGNAL_DEPRECATED),
+                     G_STRUCT_OFFSET(VteTerminalClass, text_scrolled),
+                     NULL,
+                     NULL,
+                     g_cclosure_marshal_VOID__INT,
+                     G_TYPE_NONE, 1, G_TYPE_INT);
+
+#endif /* VTE_GTK == 3 */
 
         /**
          * VteTerminal::copy-clipboard:
diff --git a/src/vtegtk.hh b/src/vtegtk.hh
index 1cb8cb49..e56314ad 100644
--- a/src/vtegtk.hh
+++ b/src/vtegtk.hh
@@ -52,10 +52,6 @@ enum {
         SIGNAL_RESIZE_WINDOW,
         SIGNAL_RESTORE_WINDOW,
         SIGNAL_SELECTION_CHANGED,
-        SIGNAL_TEXT_DELETED,
-        SIGNAL_TEXT_INSERTED,
-        SIGNAL_TEXT_MODIFIED,
-        SIGNAL_TEXT_SCROLLED,
         SIGNAL_WINDOW_TITLE_CHANGED,
         LAST_SIGNAL
 };
diff --git a/src/vteinternal.hh b/src/vteinternal.hh
index a6ad315b..e5dd7fe3 100644
--- a/src/vteinternal.hh
+++ b/src/vteinternal.hh
@@ -63,6 +63,14 @@
 #include <variant>
 #include <vector>
 
+#ifdef WITH_A11Y
+#if VTE_GTK == 3
+#include "vteaccess.h"
+#else
+#undef WITH_A11Y
+#endif
+#endif
+
 #ifdef WITH_ICU
 #include "icu-converter.hh"
 #endif
@@ -251,6 +259,11 @@ public:
 
         void unset_widget() noexcept;
 
+#ifdef WITH_A11Y
+        /* Accessible */
+        vte::glib::RefPtr<VteTerminalAccessible> m_accessible{};
+#endif
+
         /* Metric and sizing data: dimensions of the window */
         vte::grid::row_t m_row_count{VTE_ROWS};
         vte::grid::column_t m_column_count{VTE_COLUMNS};
@@ -643,10 +656,6 @@ public:
         vte::Freeable<PangoAttrList> m_im_preedit_attrs{};
         int m_im_preedit_cursor;
 
-        #ifdef WITH_A11Y
-        gboolean m_accessible_emit;
-        #endif
-
         /* Adjustment updates pending. */
         gboolean m_adjustment_changed_pending;
         gboolean m_adjustment_value_changed_pending;
@@ -1117,10 +1126,51 @@ public:
         void queue_child_exited();
         void queue_eof();
 
-        void emit_text_deleted();
-        void emit_text_inserted();
-        void emit_text_modified();
-        void emit_text_scrolled(long delta);
+#ifdef WITH_A11Y
+
+        void set_accessible(VteTerminalAccessible* accessible) noexcept
+        {
+                /* Note: The accessible only keeps a weak ref on the widget,
+                 * while GtkWidget holds a ref to the accessible already;
+                 * so this does not lead to a ref cycle.
+                 */
+                m_accessible = vte::glib::make_ref(accessible);
+        }
+
+        void emit_text_deleted() noexcept
+        {
+                if (m_accessible)
+                        _vte_terminal_accessible_text_deleted(m_accessible.get());
+        }
+
+        void emit_text_inserted()
+        {
+                if (m_accessible)
+                        _vte_terminal_accessible_text_inserted(m_accessible.get());
+        }
+
+        void emit_text_modified()
+
+        {
+                if (m_accessible)
+                        _vte_terminal_accessible_text_modified(m_accessible.get());
+        }
+
+        void emit_text_scrolled(long delta)
+        {
+                if (m_accessible)
+                        _vte_terminal_accessible_text_scrolled(m_accessible.get(), delta);
+        }
+
+#else
+
+        inline constexpr void emit_text_deleted() const noexcept { }
+        inline constexpr void emit_text_inserted() const noexcept { }
+        inline constexpr void emit_text_modified() const noexcept { }
+        inline constexpr void emit_text_scrolled(long delta) const noexcept { }
+
+#endif /* WITH_A11Y */
+
         void emit_pending_signals();
         void emit_char_size_changed(int width,
                                     int height);
@@ -1403,7 +1453,6 @@ public:
                                    vte::grid::row_t end_row,
                                    vte::grid::column_t end_col);
 
-        void subscribe_accessible_events();
         void select_text(vte::grid::column_t start_col,
                          vte::grid::row_t start_row,
                          vte::grid::column_t end_col,
diff --git a/src/widget.cc b/src/widget.cc
index d6ebfa86..6c5ca11c 100644
--- a/src/widget.cc
+++ b/src/widget.cc
@@ -27,7 +27,6 @@
 #include <stdexcept>
 #include <string>
 
-#include "cxx-utils.hh"
 #include "vtegtk.hh"
 #include "vteptyinternal.hh"
 #include "debug.h"
@@ -398,6 +397,10 @@ Widget::direction_changed(GtkTextDirection old_direction) noexcept
 void
 Widget::dispose() noexcept
 {
+#ifdef WITH_A11Y
+        m_terminal->set_accessible(nullptr);
+#endif
+
         if (m_terminal->terminate_child()) {
                 int status = W_EXITCODE(0, SIGKILL);
                 emit_child_exited(status);