Fix Dropdown issue

AGENTS.md

@@ -26,12 +26,28 @@ live in the daemon, not the GUI. Code is split across `src/` (GUI), `shared/`
 
 ## i18n — easy to get wrong
 - English is the source language (the `msgid`) **and** also has an explicit
-  identity catalog `po/en.po`. Languages: `de`, `en`, `es`, `fr`.
-- `make translations` does **not** regenerate the `.pot`. When you add or change
-  a UI string, edit `po/gkeymapper.pot` and `po/*.po` by hand, then run
-  `make translations`; validate with `msgfmt --check -o /dev/null po/<lang>.po`.
-- Never edit the generated `.mo` files under `locale/` (gitignored). Delete any
-  `po/*.po~` backups before committing.
+  identity catalog `po/en.po`. Languages: `de`, `en`, `es`, `fr`. The workflow,
+  the rules behind it and adding a language are documented in `po/README.md`; the
+  commands are `scripts/i18n.sh` (`make pot`, `make i18n-check`,
+  `make i18n-status`).
+- `make translations` does **not** regenerate the `.pot` (a pot written by a build
+  is a pot nobody reviews). When you add or change a UI string: `make pot`, then
+  `scripts/i18n.sh update` to merge it into the catalogs, then fill in the msgstrs
+  in `po/*.po` — and in `po/en.po`, whose msgstr repeats the msgid. `_()` is
+  translated where it is used, `N_()` is only collected (the help paragraphs in
+  `src/help.cpp`); the script always passes both keywords to `xgettext`.
+- The catalogs carry **no** `#: file:line` comments; the locations live in the pot
+  only (`update` merges with `msgmerge --no-location`) and
+  `scripts/i18n.sh todo <lang>` shows them from there. `make i18n-check` fails
+  when the pot is behind the sources, when a catalog misses a message or a
+  translation, or when `po/*.po` and `kLanguageCodes[]` (`src/optionsdialog.cpp`)
+  disagree — a catalog nobody can select is dead weight.
+- The daemon is **not** translated (no `setlocale`/`bindtextdomain`, only the GUI
+  sets gettext up in `main()`); `i18n-check` reports a `_()` marker under
+  `daemon/`.
+- Never edit the generated `.mo` files under `locale/` (gitignored). A manual
+  `msgmerge --update` leaves `po/*.po~` backups — the script deletes them, by hand
+  delete them before committing.
 - All documentation and source comments are English. German appears only inside
   `po/de.po`.
 
@@ -43,8 +59,8 @@ live in the daemon, not the GUI. Code is split across `src/` (GUI), `shared/`
   app id, the version and the D-Bus names both sides use. The GUI side is
   `main.cpp` (window, actions) plus `keyrow.h`, `mappings`, `capturedialog`,
   `daemonclient`, `singleinstance`, `trayicon` (StatusNotifierItem + dbusmenu),
-  `settings` (`gui.conf`), `autostart` (the session's `.desktop` entry), `help`
-  and `optionsdialog`.
+  `settings` (`gui.conf`), `autostart` (the session's `.desktop` entry), `help`,
+  `optionsdialog` and `popovers` (the popup input grab, see below).
 - The Makefile globs sources: `make` builds the GUI (`src/*.cpp` + `shared/*.cpp`
   + the generated `build/gresources.c`, see Assets) **and** the daemon
   (`daemon/*.cpp` + `shared/*.cpp`, needs `libusb-1.0-0-dev`,
@@ -120,9 +136,18 @@ live in the daemon, not the GUI. Code is split across `src/` (GUI), `shared/`
   with `SIGTERM` after checking that the bus name really belongs to a
   `gkeymapper` process. Only then does the new process become the primary,
   because otherwise `GtkApplication` would hand over to the old process and its
-  window — possibly one from an older build — would stay on screen. Row edits
-  that were not saved are gone with the replaced window; there is no implicit
-  save.
+  window — possibly one from an older build — would stay on screen.
+- The takeover also carries the rows over, so an edit that was typed but never
+  saved is not lost with the replaced window: before the quit,
+  `replace_running_instance()` asks for them (`Window.GetRows` at
+  `ipc::kWindowObjectPath`) and `build_ui()` starts the new window from that text
+  instead of the file. The answering side is `serve_rows_handover()` — called from
+  `on_activate` **after** `build_ui()`, since that is what fills the rows — with
+  `mappings_text(rows)`, i.e. the mappings file format, so both ends use
+  `src/mappings`. An instance that does not answer (an older build) leaves the new
+  window with the file; asking and serving are best effort and never fatal. The
+  file itself is untouched until the user presses `Save mappings` — the handover
+  carries row state, it does not save.
 - `on_activate` presents the window that is already there
   (`gtk_application_get_windows()`), it does not build a second one: a remote
   activation — an older build starting next to a new one — would otherwise add a
@@ -168,9 +193,17 @@ live in the daemon, not the GUI. Code is split across `src/` (GUI), `shared/`
   deleting it. The `Exec` line is the binary that is running now
   (`g_file_read_link("/proc/self/exe")`, quoted, falls back to the name from
   `PATH`), so an enabled switch starts the build the user was looking at, and the
-  `Comment` reuses the About text (no second sentence for translators). Only the
-  **window** is started; the daemon is a systemd user service and stays untouched
-  — the GUI is a convenience, the daemon is the part that has to run.
+  `Comment` reuses the About text (no second sentence for translators). That
+  `Exec` line also carries `--start-hidden` (`kStartHiddenArg`, taken off the
+  command line in `main()`), so the window is built but stays in the tray: a login
+  does not put it in front of the user, while a start by hand (no flag) always
+  shows it. Keeping it hidden is only safe while the panel really hosts the icon,
+  and only the panel can answer that, asynchronously — so
+  `on_start_hidden_timeout()` shows the window after `kStartHiddenGraceMs` if the
+  icon never turned up. A rebuild (language switch) comes up the way the window
+  was left: `build_ui()` takes the visibility as an argument. Only the **window**
+  is started; the daemon is a systemd user service and stays untouched — the GUI
+  is a convenience, the daemon is the part that has to run.
 - `Minimize to tray` hides the window on close (`"close-request"` handler) and on
   minimize. GTK4 has **no** minimize signal and `GdkToplevel` exposes no
   properties, so `gdk_toplevel_get_state()` is polled every 250 ms while the
@@ -222,6 +255,30 @@ live in the daemon, not the GUI. Code is split across `src/` (GUI), `shared/`
   `GtkScrolledWindow` (one collapsed the rows earlier). Keep the fixed widths
   (Type dropdown 150 px, target entry 440 px min, action button 180 px) so rows
   do not jump when the language changes.
+- Both dropdowns (Type in the main window, Language in the options window) give
+  every entry a **trailing space**, added where the model is built — the msgids
+  stay the bare words. `GtkDropDown` appends the check mark of the selected entry
+  directly behind the text (`setup_item()` in `gtkdropdown.c`), and without that
+  space it touches the last letter. The selection is read back by index, never by
+  the string, so the space is harmless everywhere else.
+- `src/popovers.{h,cpp}` takes the input grab off the popovers of a window (the
+  dropdowns' popup lists and the header bar menu) and closes them itself: on a
+  click in the window outside the popup, on Escape, and when the window loses the
+  focus. GTK 4.6 grabs the pointer for an open popover, an X11 grab swallows the
+  clicks of **every** other client, and this GTK does not close the popup on a
+  click outside the application either — with one dropdown open nothing else
+  could be clicked any more, not another program and not the desktop, and the
+  popup stayed open until Escape (measured with a plain GTK4 probe: the click on
+  the bare desktop never reached it and `_NET_ACTIVE_WINDOW` did not change; a
+  click inside the app's own window did close the popup). GTK3 replayed such a
+  click to the window under the pointer, GTK4 dropped that. `autohide` is what
+  installs the grab, so switching it off hands the clicks back to the desktop;
+  the dismissal is then done by hand, which is the rest of the module. It runs
+  once per toplevel window (`build_ui`, `OptionsDialog::show`), notices popovers
+  GTK creates later on the press that opens them, and leaves the press on the
+  popup itself alone — that one is an entry choice and must reach the entry
+  (recognised by its own surface, not by its position, because GTK reports it in
+  the popup's coordinates).
 - Because it is pure GTK4 it follows the system theme (Mint-Y ships a
   `gtk-4.0` theme). Do **not** add the `suggested-action` CSS class — Mint-Y
   tints it green and it clashes with the rest of the layout.
@@ -415,7 +472,11 @@ replaced process exits `0` when it was closed through the D-Bus `quit` action, o
 instance from an older commit (`git worktree add /tmp/legacy <commit>`), start it
 and then start the current build next to it. Starting the GUI twice also covers
 the `activate` guard: without it the second start adds a second window to the
-running process.
+running process. The rows come along (see the takeover notes above), which is
+checkable the same way: type into a row of the first instance without saving,
+start the second one and read its rows back (next bullet) — they show the edit,
+while the mappings file still has the old content. An instance from an older
+commit cannot hand rows over, so there the new window shows the file.
 
 The footer's daemon line is a plain `GtkLabel`, so its text can be read back over
 AT-SPI: `<app> → frame → panel → label` whose name starts with the translated
@@ -433,7 +494,12 @@ is up. Three notes for tests:
   `import -window $(wmctrl -l | grep 'gKeyMapper – Logitech' | awk '{print $1}')
   /tmp/gui.png`. The footer label works over AT-SPI (its accessible *name* is the
   text), but a screenshot of the cropped footer is easier and shows what the user
-  sees.
+  sees. Easier than a screenshot for the rows themselves: the running instance
+  answers `gdbus call --session --dest dev.rager.gkeymapper --object-path
+  /dev/rager/gkeymapper/Window --method dev.rager.gkeymapper.Window.GetRows` with
+  exactly the mappings text its rows hold (`G1=command:/bin/true…`). It is the
+  same channel the takeover uses, so it also tells whether an edit is still
+  unsaved: compare it with the file.
 - `XTest`-synthesized clicks need the window mapped and focused; the `Save`
   button is `push button` + name `Mappings speichern` in the German locale.
 
@@ -447,12 +513,24 @@ The tray icon, the help and the options window need neither hardware nor a click
   pid, two registrations) means the "register once" rule broke.
 - Its menu is readable without the panel:
   `gdbus call --session --dest org.kde.StatusNotifierItem-<pid>-1 --object-path
-  /MenuBar --method com.canonical.dbusmenu.GetLayout 0 -1` returns the root node
-  with the two children. Drive a click the way the panel does — **batched**, as
-  `EventGroup`, because that is the path a real click takes:
+  /MenuBar --method com.canonical.dbusmenu.GetLayout -- 0 -1 "[]"` returns the
+  root node with the two children (the `--` keeps gdbus from reading the `-1` as
+  an option, and the empty array is the property-name list `GetLayout` expects).
+  Drive a click the way the panel does — **batched**, as `EventGroup`, because
+  that is the path a real click takes:
   `gdbus call … --method com.canonical.dbusmenu.EventGroup
-  "[(1, 'clicked', <'', >, 0)]"` shows the window, id `2` quits the program.
-  A single `Event` call works too but is not what the watcher sends.
+  "[(1, 'clicked', <''>, 0)]"` shows the window, id `2` quits the program (the
+  empty string is the variant `v` of the event; `<''>` is the only spelling gdbus
+  accepts). A single `Event` call works too but is not what the watcher sends.
+- A start with `--start-hidden` (what the autostart entry passes) is checkable the
+  same way: no window in `wmctrl -lp` while the item above is registered and the
+  process stays, and the menu's `Show` brings the window up. Without a tray watcher
+  the window has to appear by itself after ~5 s — on a private bus that is awkward
+  to see, because a fresh `dbus-daemon` makes GTK's startup wait on gvfs/portal
+  services (`GIO_USE_VFS=local` unblocks it, and it can still take ~20 s), so
+  `on_activate` runs much later than on the session bus. Do not read "no window" as
+  the flag working until the tray item or the window object
+  (`Window.GetRows`) proves the app got that far.
 - The watcher is fragile by construction: it walks the layout it mirrors, so a
   malformed node (bare ids instead of full nodes) aborts it. After a change to
   the layout, check it survived (`pgrep -a xapp-sn-watcher`) and that no new
@@ -482,6 +560,22 @@ The tray icon, the help and the options window need neither hardware nor a click
   One pitfall while clicking: the options window is transient for the main
   window, so it always stays above it — clicks meant for the main window hit the
   options window unless it is closed first.
+- The popups (`src/popovers.cpp`) are checkable without hardware, and a click
+  test has to know two things: a popup is an override-redirect window that
+  disappears from `xwininfo`'s map state (or a `viewable.py`-style walk) but never
+  appears in `wmctrl -l`, and a click on the dropdown that is *already* open
+  closes it again — so "click the dropdown" does not mean "the popup is open",
+  always read the map state. What to check: with a type dropdown open, a click on
+  a *different* window must arrive there (`_NET_ACTIVE_WINDOW` changes to that
+  client, which proves the press was delivered and not swallowed) and the popup
+  must be gone; the same for a click on the bare desktop; a click in the window
+  outside the popup closes it without changing the focus; Escape closes it; a
+  click on an entry closes it and changes the row (`Window.GetRows`, e.g.
+  `G1=key:…` becomes `G1=command:…`); a click on the header bar's menu button
+  opens the menu and behaves like the dropdown. Every synthetic click must be
+  guarded by the id of the window that is expected to be topmost — the popup is
+  not in `_NET_CLIENT_LIST_STACKING`, so a click aimed at the popup has to be
+  checked against the window *below* it.
 - The autostart switch needs no click to read and one click to write, because the
   `.desktop` entry *is* the state: put `$HOME/.config/autostart/gkeymapper.desktop`
   there (or delete it), close the options window (`wmctrl -i -c <id>`) and