Fix selected row not being restored across content reloads - #2318
Open
vredesbyyrd wants to merge 1 commit into
Open
Fix selected row not being restored across content reloads#2318vredesbyyrd wants to merge 1 commit into
vredesbyyrd wants to merge 1 commit into
Conversation
Collaborator
|
Looks clean. Thanks. I'll check it out when I have some time. |
DaveDavenport
self-requested a review
August 2, 2026 10:56
rofi_view_set_selected_line() resolves the line against line_map, which only describes content the view has already loaded. A mode that reloads and selects in one go cannot satisfy that: rofi_view_reload() arms the backend's reload timer, and line_map is rebuilt later, in rofi_view_refilter_real(). The lookup therefore ran against the previous list and fell back to row 0 whenever the requested line was out of its range, leaving rofi_view_get_selected_line() and listview_get_selected() disagreeing. Record such a request in state->pending_selected_line instead of falling back to row 0, and apply it in refilter, in the same pass that installs the new content and before it repaints, so no frame shows the wrong row. Requests are one-shot and honoured only on the reload path: state->selected_line is stale between activations, so re-applying it on an ordinary refilter would move the selection away from where the user left it. Script mode's keep-selection is affected: it bounds-checks new-selection against the new list length, then resolves it against the old line_map.
vredesbyyrd
force-pushed
the
fix-pending-selected-line
branch
from
August 9, 2026 02:47
d374dfa to
974b0a3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
When a mode reloads its entries and requests that a particular row be selected, rofi sometimes selects row 0 instead. The failure depends on the size of the list being replaced rather than the new one, so it appears arbitrary: after navigating from a 100-item list into a 5-row submenu and back, restoring index 4 works, while index 5, one past the end of the list being replaced, falls back to row 0.
rofi_view_set_selected_line()resolves the requested line againststate->line_map, which describes the content the view currently has loaded. A mode producing new content plus a selection to go with it cannot satisfy that precondition:rofi_view_reload()only arms the backend's reload timer, andline_mapis rebuilt later, inrofi_view_refilter_real(). So the lookup runs against the previous page, and when the requested index is not in it the loop falls through toselected = 0.state->selected_linestill reports the requested line, leavingrofi_view_get_selected_line()andlistview_get_selected()disagreeing.Because failure depends on the requested index happening to be in range of the page still on screen, it presents as an arbitrary threshold rather than an ordering problem, which is most of why it took me so long to pin down, or I just have brain-rot :/
Reproducing
Built-in script mode hits this.
script_mode_result()bounds-checksnew-selectionagainst the new list length and then resolves it against the oldline_map(source/modes/script.c:431):Debug script and instructions: https://gist.github.com/vredesbyyrd/a8fe01a182f49e32987e1ea394e3359b
Two nested menus, 100 rows and 5 (the submenu's first row is
< Back). Select row 5, then Back: the selection returns to row 5. Select row 6, then Back: the selection lands on row 0, that request is for index 5, resolved against the 5-row submenu still on screen. The boundary is the size of the list you navigate away from. Fails on both backends (wayland/x11); fixed by this PR.Where I hit it
rofi-blocks, writing a script with nested menus (artists → albums → tracks) that restores the row you came from on the way back up.
There it only ever manifested on wayland, which tracing eventually explained too. rofi-blocks carries a workaround that re-issues the selection from a 33 ms timeout (OmarCastro/rofi-blocks@073b4c8, for OmarCastro/rofi-blocks#31). xcb arms its reload timer at 10 ms (
source/xcb/view.c:460) and wayland at ~67 ms (source/wayland/view.c:307), so that retry lands after the line map has been rebuilt on xcb and before it on wayland. On xcb the first frame is still drawn with the wrong row and the retry corrects it a frame later, which is presumably why it never looked like a bug there, though it did occasionally read as a glitch.The change
state->pending_selected_lineto record a selection requestrofi_view_set_selected_line()could not yet resolve.rofi_view_refilter_real()applies the pending request immediately afterlistview_set_num_elements()and before repainting, so the first frame with the new content already has the correct row selected. One step, no timer.state->selected_lineis deliberately stale between activations (plain listview navigation never writes it), so re-applying it on an ordinary filter refilter would move the selection out from under the user.rofi_view_select_line(), shared by both sites.-selected-rowis unaffected: it runs afterrofi_view_create()has already refiltered, with the selection still at 0.rofi_view_clear_input()is unaffected because both its call sites immediately force a reload throughrofi_view_switch_mode(), which resolves the request in that same pass.Testing
meson testpasses. I testeddrun,run,window,dmenu, andscriptwith filtering, page navigation,-selected-row, and mode switching on both backends against a stock build and observed no regressions.Related
With this in place the rofi-blocks workaround becomes unnecessary and can be dropped (vredesbyyrd/rofi-blocks@17525c5).
That removal is only correct against a rofi carrying this fix; against an older rofi the behaviour is the pre-workaround one. I have not proposed it upstream to rofi-blocks yet, it seemed right to see how this lands first.
Full Disclosure
I spent a considerable amount of time investigating and narrowing this down, unsuccessfully, before bringing in any llm assistance. I am not a
Cexpert and I am not deeply familiar with rofi's code, so I used "AI" to help me understand the code paths involved and to implement the fix. Totally understand if you would prefer not to accept llm-assisted contributions. I'm filing it because the issue seemed worth reporting either way, and because I have been running the patch with rofi-blocks without hitting any regressions in my own testing.