From f00d1619006471ada6e55d1d34d0d596d6037cda Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Mon, 12 Oct 2026 15:10:22 +0200 Subject: [PATCH] fix: row titles mean their mnemonics instead of drawing the underscore Fifteen display strings in the two preference forms were written as GTK mnemonics -- `_Directory`, `_Server`, `Multicast _group`, `_Theme` and the rest -- but the property that makes GTK read them was never set, so GTK drew the underscore literally: `_Directory` in the row, not a D with a line under it. Two switches are involved, FALSE by default in each case. `GtkLabel`/`GtkButton` take `use-underline`; the rows take `AdwPreferencesRow:use-underline`, "whether an embedded underline in the title indicates a mnemonic". The two buttons in the source form already set it (so their mnemonics worked and showed no underscore); the thirteen rows did not. All thirteen now do. The row classes that carry the property are not all related -- AdwEntryRow has its own, while AdwSpinRow, AdwComboRow and AdwSwitchRow get it through AdwActionRow -- so the probe reads it by name on a GObject (g_object_class_find_property, then g_object_get) rather than casting to one row type; a cast to a `GtkWidget*` where the type is only forward-declared does not compile at all. `_Cancel` and `_Connect` both claimed C in the same form, so Alt+C could only reach one of them: that marker was unreachable, which is worse than absent because it is in the source to be trusted. `_Connect` becomes `Conn_ect`, freeing C for Cancel. [prefs-probe] reads the count (4/4) on the real widgets; check_ui validates the property against the GIR for every object in every .ui file, inherited properties included. --- docs/claude/invariants-gtk-and-chrome.md | 18 ++++++++++++++++++ docs/claude/probe-readings.md | 13 +++++++++++++ src/source_dialog.cpp | 7 +++++++ src/ui/preferences.ui | 5 +++++ src/ui/source-form.ui | 10 +++++++++- src/ui_app.hpp | 3 +++ src/ui_prefs.hpp | 17 +++++++++++++++++ src/ui_probe.cpp | 6 ++++++ 8 files changed, 78 insertions(+), 1 deletion(-) diff --git a/docs/claude/invariants-gtk-and-chrome.md b/docs/claude/invariants-gtk-and-chrome.md index 51b3b82..8d3c546 100644 --- a/docs/claude/invariants-gtk-and-chrome.md +++ b/docs/claude/invariants-gtk-and-chrome.md @@ -280,3 +280,21 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear re-enter a dead window. GTK does not promise to take a transient down with its parent, and a window whose handlers point into a returned stack frame is how this project already earned one use-after-free. + +- **A `_` in a label is a mnemonic only on a widget that was asked for one.** GTK reads the underscore as + "the next letter is this widget's accelerator, and draw it underlined", so wherever the widget has not + been asked, GTK draws the underscore *literally* -- which is what `_Directory`, `_Server`, `Keep _events` + and eleven more titles did in the two preference forms, for as long as they existed. Two different + switches, FALSE by default in both cases: `GtkLabel`/`GtkButton` take `use-underline`, and the rows take + **`AdwPreferencesRow:use-underline`** ("whether an embedded underline in the title indicates a mnemonic"). + The row classes that carry it are not all related -- `AdwEntryRow` has its own, while + `AdwSpinRow`/`AdwComboRow`/`AdwSwitchRow` get it through `AdwActionRow` -- so `row_underlines()` asks by + name on a `GObject` (`g_object_class_find_property`, then `g_object_get`) instead of casting to one row + type: casting a row whose type is only forward-declared in that translation unit does not even compile. + `test-ui` is what catches the property being set on a class that does not have it -- it validates every + property against the GIR, inherited ones included. +- **A mnemonic is unique per window, and the first match wins.** `_Cancel` and `_Connect` both claimed C in + one form, so `Alt+C` could only ever reach one of them and the other's marker was unreachable -- worse + than no mnemonic, because the underscore is there in the source for a reader to trust. `Conn_ect` freed + it. Rows on different pages of a preferences dialog may share a letter: an unshown page cannot be + activated. diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index 46ea473..e350608 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -1143,3 +1143,16 @@ copy and never writes is exactly the failure this has to catch. default path back into the field and presses *Apply*, and the file must end up with **no** `event_cache_dir` at all. A path in the file is a user's choice, and typing the default is not one -- pinning it there would stop the setting following the cache directory it is named after. + +## `mnemonics=` in the two preference probes + +``` +[prefs-probe] mnemonics=4/4 +[sources] form mnemonics=8/8 +``` + +How many of the dialog's rows have `use-underline` set -- the property that makes a `_` in a row's title a +mnemonic. It is FALSE on `AdwPreferencesRow` by default, which is why these titles were drawn with a +literal underscore; anything below the full count is that bug back. Read by property name rather than +through one row type, for the reason in `invariants-gtk-and-chrome.md`. The form's line appears only when +`UDPSCOPE_PROBE_NEWHOST` drives the form. diff --git a/src/source_dialog.cpp b/src/source_dialog.cpp index c245716..68d0085 100644 --- a/src/source_dialog.cpp +++ b/src/source_dialog.cpp @@ -642,6 +642,13 @@ gboolean pick_probe(gpointer user) { gtk_editable_set_text(GTK_EDITABLE(c.form_note), "made by the new-host probe"); adw_spin_row_set_value(c.form_buffer, 32); adw_switch_row_set_active(c.form_relative, TRUE); + // 8/8: the form's row titles are mnemonics (`_Server`, `_Port`, `Multicast _group`, ...). The two + // buttons already asked for it; the rows did not, and drew their underscores. + std::fprintf(stderr, "[sources] form mnemonics=%d/8\n", + (row_underlines(G_OBJECT(c.form_server)) + row_underlines(G_OBJECT(c.form_port)) + + row_underlines(G_OBJECT(c.form_group)) + row_underlines(G_OBJECT(c.form_iface)) + + row_underlines(G_OBJECT(c.form_name)) + row_underlines(G_OBJECT(c.form_note)) + + row_underlines(G_OBJECT(c.form_buffer)) + row_underlines(G_OBJECT(c.form_relative)))); std::fprintf(stderr, "[sources] form: name='%s' note='%s' buffer=%.6g relative=%d " "connect_sensitive=%d\n", diff --git a/src/ui/preferences.ui b/src/ui/preferences.ui index 74d95db..3c84219 100644 --- a/src/ui/preferences.ui +++ b/src/ui/preferences.ui @@ -32,12 +32,14 @@ _Directory + True One directory in which every source gets a folder of its own, never one directory between two sources Keep _events + True Per source; 0 keeps every event, which is a decision about the disk @@ -62,12 +64,14 @@ _Relative time + True Take the plot's time from the sample rate and the packet counter rather than from the stream's own clock, so arrival and clock jitter cannot move a sample; absolute time is still kept for export. Read at start-up, because the buffers are planned before any source is known. _Theme + True What the interface looks like; auto follows the system @@ -84,6 +88,7 @@ _Reset to defaults + True view-refresh-symbolic diff --git a/src/ui/source-form.ui b/src/ui/source-form.ui index 35c9430..c21c1ad 100644 --- a/src/ui/source-form.ui +++ b/src/ui/source-form.ui @@ -38,7 +38,7 @@ - _Connect + Conn_ect 1 1 suggested-action @@ -59,11 +59,13 @@ _Server + True _Port + True 1 @@ -77,11 +79,13 @@ Multicast _group + True _Interface + True @@ -93,16 +97,19 @@ _Name + True N_ote + True _Buffer MB + True sample buffer per source, applied when this source is opened @@ -117,6 +124,7 @@ _Relative time + True plot time from the packet counter and the declared rate, instead of the stream's own time signal diff --git a/src/ui_app.hpp b/src/ui_app.hpp index 9f264da..75f97c2 100644 --- a/src/ui_app.hpp +++ b/src/ui_app.hpp @@ -13,6 +13,7 @@ #pragma once + #include #include "acquisition.hpp" @@ -33,6 +34,8 @@ #include "status_text.hpp" #include "waveform.hpp" + + #include #include diff --git a/src/ui_prefs.hpp b/src/ui_prefs.hpp index 4004818..69be0ae 100644 --- a/src/ui_prefs.hpp +++ b/src/ui_prefs.hpp @@ -29,6 +29,23 @@ #include "settings.hpp" +/// Whether a row asks for `use-underline` -- the property (default FALSE) that makes a `_` in a row's title +/// a mnemonic. Without it GTK draws the underscore as a literal character, which is what `_Directory` and the +/// other row titles did. +/// +/// Asked for by *name* on a plain `GObject`: the row classes that carry the property are not all related +/// (`AdwEntryRow`'s `use-underline` is its own, `AdwSpinRow`/`AdwComboRow`/`AdwSwitchRow`'s come through +/// `AdwActionRow`), so a common getter or a cast to one row type would miss half of them -- and passing a +/// row as a `GtkWidget*` where the type is only forward-declared does not even compile. A class without the +/// property answers false rather than crashing. +inline bool row_underlines(GObject* row) { + if (row == nullptr) return false; + if (g_object_class_find_property(G_OBJECT_GET_CLASS(row), "use-underline") == nullptr) return false; + gboolean on = FALSE; + g_object_get(row, "use-underline", &on, nullptr); + return on != FALSE; +} + namespace udpscope { namespace ui_impl { diff --git a/src/ui_probe.cpp b/src/ui_probe.cpp index a495875..095053e 100644 --- a/src/ui_probe.cpp +++ b/src/ui_probe.cpp @@ -1696,6 +1696,12 @@ gboolean prefs_probe(gpointer user) { // the `.ui` -- the one mistake neither the build nor `GtkBuilder` sees is a missing id. const char* shown = a.prefs.dir_row != nullptr ? gtk_editable_get_text(GTK_EDITABLE(a.prefs.dir_row)) : "-"; + // 4/4: every row's title carries a mnemonic (`_Directory`, `Keep _events`, ...), so a row that does not + // ask for `use-underline` draws the underscore instead of underlining the letter. + std::fprintf(stderr, "[prefs-probe] mnemonics=%d/4\n", + (row_underlines(G_OBJECT(a.prefs.dir_row)) + row_underlines(G_OBJECT(a.prefs.keep_row)) + + row_underlines(G_OBJECT(a.prefs.relative_row)) + + row_underlines(G_OBJECT(a.prefs.theme_row)))); std::fprintf(stderr, "[prefs-probe] opened=%d rows=%d dir=%s store=%s\n", a.prefs.dialog != nullptr ? 1 : 0, a.prefs.dir_row != nullptr ? 1 : 0, a.prefs.dir_row != nullptr ? shown : "-",