serrebi/BlindRSS

Watch

Fork 0

mirror of https://github.com/serrebidev/BlindRSS.git synced 2026-08-13 11:59:26 -07:00

Regression: Full article text remains in the read-only preview area #64

New issue

Closed

opened 2026-07-24 08:29:10 -07:00 by serrebi · 2 comments

serrebi commented

2026-07-24 08:29:10 -07:00

Owner

Affected Versions

Versions: about 1.119.0 – 1.120.3 (but that's not certain).

Preconditions

In Settings > Feeds & Articles, the Rich full-text view checkbox is unchecked.

Steps to Reproduce

Open the list of articles for any feed.
Select an article.
Press Tab to move focus to the preview area displaying the extracted full text of the article.
Press Shift+Tab to return focus to the list of articles.
Select a different article in the list.
Press Tab to move focus to the preview area for the newly selected article.

Actual Result

On step 6, in some cases, the preview area still shows the full text from the article selected on step 3 instead of the text for the currently selected article.

Expected Result

The preview area should always display either the brief description from the feed or the extracted full text corresponding to the currently selected article in the list.
Text from previously viewed articles should never remain in the preview area.

Additional Notes

The bug is non-deterministic and occurs intermittently.
Restarting the application clears the stale text.
No specific conditions beyond the preconditions are known to consistently trigger the issue.

In versions between 1.119.0 and 1.120.3 (but that's not certain), a regression has appeared where the full text of an article loaded into the preview area stays there even after the user selects a different article. The issue is intermittent: it does not occur consistently, and no definitive set of steps reliably reproduces it. The problem is only resolved by restarting the application. #### Affected Versions - Versions: about 1.119.0 – 1.120.3 (but that's not certain). #### Preconditions - In Settings > Feeds & Articles, the Rich full-text view checkbox is unchecked. #### Steps to Reproduce 1. Open the list of articles for any feed. 2. Select an article. 3. Press `Tab` to move focus to the preview area displaying the extracted full text of the article. 4. Press `Shift+Tab` to return focus to the list of articles. 5. Select a different article in the list. 6. Press `Tab` to move focus to the preview area for the newly selected article. #### Actual Result - On step 6, in some cases, the preview area still shows the full text from the article selected on step 3 instead of the text for the currently selected article. #### Expected Result - The preview area should always display either the brief description from the feed or the extracted full text corresponding to the currently selected article in the list. - Text from previously viewed articles should never remain in the preview area. #### Additional Notes - The bug is non-deterministic and occurs intermittently. - Restarting the application clears the stale text. - No specific conditions beyond the preconditions are known to consistently trigger the issue.

serrebi commented

2026-07-24 08:29:20 -07:00

Author

Owner

Thanks @tseykovets — reproduced the mechanism. Your instinct that it's non-deterministic is right, but there is a hard trigger condition behind it, and it explains why only a restart clears it.

Regression window: 0c4fa73 ("fix: reduce NVDA lag in YouTube readers", v1.120.3), which added _swap_focused_large_reader and the _reader_displayed_text memo in gui/mainframe.py.

The trigger

Three conditions have to line up, which is why it's intermittent:

Classic reader (Rich full-text view off) — your precondition.
The extracted full text is ≥ 16384 characters (LARGE_READER_TEXT_CHARS).
Focus is inside the reader when the extraction lands — i.e. exactly your step 3/6 Tab.

Short articles never take this path, which is why no fixed set of steps reproduces it.

What happens

To hand NVDA a fully-populated RichEdit in one focus transition, _swap_focused_large_reader builds a new wx.TextCtrl, swaps it into the sizer, points self.content_ctrl at it, and calls new.SetFocus().

That SetFocus() fires EVT_SET_FOCUS synchronously → on_content_focus → _schedule_fulltext_load_for_index(idx, force=True). The text was already cached by _fulltext_apply_result before it applied it, and a successful web extraction is cache_source == "web", so _cached_fulltext_is_authoritative passes and the cache-hit branch calls _set_article_reader_text synchronously, re-entering the function that is still mid-swap.

The memo that would short-circuit that re-entry (_reader_displayed_text) is assigned only after the swapper returns (mainframe.py:5257-5258), so on re-entry it still holds the previous article's text, the guard misses, and it swaps again. Each turn builds another TextCtrl holding the same multi-megabyte string.

I bound the real _set_article_reader_text and _swap_focused_large_reader to a stub reproducing that call graph. It recurses without bound — it only stopped at depth 61 because I capped it:

max reentrancy depth : 61
controls created     : 61
visible controls     : [Ctrl#59, Ctrl#60]

Why the stale text is permanent

This is the part that answers "only a restart fixes it". The swap mutates shared state before the risky call and rolls nothing back on failure:

self.content_ctrl = new     # already reassigned
new.SetFocus()              # <-- raises (RecursionError, or anything in the focus handler)
old.Hide()                  # never runs
wx.CallAfter(old.Destroy)   # never runs

The except Exception handler only destroys new if new is not self.content_ctrl — and new is self.content_ctrl by then, so nothing is undone. The old control stays visible and parented, showing the full text it was last given, while self.content_ctrl points at a different control. Every later write — including the "Loading..." reset and the next article's description — goes to the control you can no longer see. The pane is frozen on the previous article's text for the rest of the session. My stub run ends in exactly that state: two visible controls, content_ctrl pointing at the wrong one.

Fix direction

Assign _reader_displayed_text before invoking the swapper, so the re-entrant call short-circuits, and add an explicit reentrancy guard around the swap.
Make the swap's failure path roll back self.content_ctrl and the sizer, or do the SetFocus() last with the old control already retired.
Separately: the memo is bypassed by the direct content_ctrl.SetValue(...) writes (mainframe.py:1579, 1627, 1684, 9593, 9609, 11100) which leave it describing text the control no longer holds. Those should invalidate it.

Note there's no test coverage for _swap_focused_large_reader — tests/test_reader_performance.py only exercises the reader_performance.py helpers, not the mainframe swap, which is how this got through.

Happy to put up a PR with the fix and a regression test that asserts the swap is non-reentrant and leaves exactly one visible control on failure — say the word.

Thanks @tseykovets — reproduced the mechanism. Your instinct that it's non-deterministic is right, but there is a hard trigger condition behind it, and it explains why only a restart clears it. **Regression window:** `0c4fa73` ("fix: reduce NVDA lag in YouTube readers", v1.120.3), which added `_swap_focused_large_reader` and the `_reader_displayed_text` memo in `gui/mainframe.py`. ### The trigger Three conditions have to line up, which is why it's intermittent: 1. Classic reader (Rich full-text view off) — your precondition. 2. The extracted full text is **≥ 16384 characters** (`LARGE_READER_TEXT_CHARS`). 3. Focus is **inside the reader** when the extraction lands — i.e. exactly your step 3/6 Tab. Short articles never take this path, which is why no fixed set of steps reproduces it. ### What happens To hand NVDA a fully-populated RichEdit in one focus transition, `_swap_focused_large_reader` builds a *new* `wx.TextCtrl`, swaps it into the sizer, points `self.content_ctrl` at it, and calls `new.SetFocus()`. That `SetFocus()` fires `EVT_SET_FOCUS` synchronously → `on_content_focus` → `_schedule_fulltext_load_for_index(idx, force=True)`. The text was already cached by `_fulltext_apply_result` before it applied it, and a successful web extraction is `cache_source == "web"`, so `_cached_fulltext_is_authoritative` passes and the cache-hit branch calls `_set_article_reader_text` **synchronously, re-entering the function that is still mid-swap**. The memo that would short-circuit that re-entry (`_reader_displayed_text`) is assigned only *after* the swapper returns (`mainframe.py:5257-5258`), so on re-entry it still holds the previous article's text, the guard misses, and it swaps again. Each turn builds another TextCtrl holding the same multi-megabyte string. I bound the real `_set_article_reader_text` and `_swap_focused_large_reader` to a stub reproducing that call graph. It recurses without bound — it only stopped at depth 61 because I capped it: ``` max reentrancy depth : 61 controls created : 61 visible controls : [Ctrl#59, Ctrl#60] ``` ### Why the stale text is permanent This is the part that answers "only a restart fixes it". The swap mutates shared state *before* the risky call and rolls nothing back on failure: ```python self.content_ctrl = new # already reassigned new.SetFocus() # <-- raises (RecursionError, or anything in the focus handler) old.Hide() # never runs wx.CallAfter(old.Destroy) # never runs ``` The `except Exception` handler only destroys `new` `if new is not self.content_ctrl` — and `new` *is* `self.content_ctrl` by then, so nothing is undone. The old control stays **visible and parented**, showing the full text it was last given, while `self.content_ctrl` points at a different control. Every later write — including the "Loading..." reset and the next article's description — goes to the control you can no longer see. The pane is frozen on the previous article's text for the rest of the session. My stub run ends in exactly that state: two visible controls, `content_ctrl` pointing at the wrong one. ### Fix direction 1. Assign `_reader_displayed_text` **before** invoking the swapper, so the re-entrant call short-circuits, and add an explicit reentrancy guard around the swap. 2. Make the swap's failure path roll back `self.content_ctrl` and the sizer, or do the `SetFocus()` last with the old control already retired. 3. Separately: the memo is bypassed by the direct `content_ctrl.SetValue(...)` writes (`mainframe.py:1579`, `1627`, `1684`, `9593`, `9609`, `11100`) which leave it describing text the control no longer holds. Those should invalidate it. Note there's no test coverage for `_swap_focused_large_reader` — `tests/test_reader_performance.py` only exercises the `reader_performance.py` helpers, not the mainframe swap, which is how this got through. Happy to put up a PR with the fix and a regression test that asserts the swap is non-reentrant and leaves exactly one visible control on failure — say the word.

serrebi closed this issue

2026-07-24 08:50:24 -07:00

serrebi commented

2026-07-24 08:50:24 -07:00

Author

Owner

Fixed and shipped in v1.120.5 — https://github.com/serrebidev/BlindRSS/releases/tag/v1.120.5

Windows installer and portable ZIP are up now; the macOS/Linux assets are attaching from the Actions build. Auto-update will offer it.

Fix is 420ebb4, three changes on top of the diagnosis above:

_swap_focused_large_reader refuses to run re-entrantly, so the SetFocus() → on_content_focus → cache-hit path can no longer start a second swap inside the first. The guard is released in a finally, so a failed attempt can't wedge every later swap.
A failed swap now rolls back completely — sizer restored, content_ctrl pointed back at the surviving control, replacement destroyed. This was the part that made it permanent: previously a half-applied swap left the old control visible while writes went somewhere else. The article text still reaches the reader through the normal path when the swap is skipped, since the swap is only a latency optimisation.
_reader_displayed_text is now recorded before the swap, cleared if a write raises, and invalidated at the ten direct content_ctrl writes that used to leave it describing text the control no longer held.

New regression tests in tests/test_reader_swap_reentrancy.py cover non-reentrancy, rollback, guard release, and memo invalidation. Four of the five fail against the old build — the runaway one built 10 replacement controls where 2 is correct — so they pin the actual bug rather than just the fixed behaviour. Full suite: 1970 passed, 7 skipped.

One thing worth saying plainly: because you couldn't pin down reliable steps, it would have been easy to file this as unreproducible. The steps you did give were exactly right — the Tab into the preview is what puts focus in the reader when the extraction lands, which is one of the three conditions. The other two are the classic reader and an article whose extracted text runs over 16384 characters, which is why it only ever hit some articles.

Please do reopen if you see any stale text after updating.

Fixed and shipped in **v1.120.5** — https://github.com/serrebidev/BlindRSS/releases/tag/v1.120.5 Windows installer and portable ZIP are up now; the macOS/Linux assets are attaching from the Actions build. Auto-update will offer it. Fix is `420ebb4`, three changes on top of the diagnosis above: 1. `_swap_focused_large_reader` refuses to run re-entrantly, so the `SetFocus()` → `on_content_focus` → cache-hit path can no longer start a second swap inside the first. The guard is released in a `finally`, so a failed attempt can't wedge every later swap. 2. A failed swap now rolls back completely — sizer restored, `content_ctrl` pointed back at the surviving control, replacement destroyed. This was the part that made it permanent: previously a half-applied swap left the old control visible while writes went somewhere else. The article text still reaches the reader through the normal path when the swap is skipped, since the swap is only a latency optimisation. 3. `_reader_displayed_text` is now recorded *before* the swap, cleared if a write raises, and invalidated at the ten direct `content_ctrl` writes that used to leave it describing text the control no longer held. New regression tests in `tests/test_reader_swap_reentrancy.py` cover non-reentrancy, rollback, guard release, and memo invalidation. Four of the five fail against the old build — the runaway one built 10 replacement controls where 2 is correct — so they pin the actual bug rather than just the fixed behaviour. Full suite: 1970 passed, 7 skipped. One thing worth saying plainly: because you couldn't pin down reliable steps, it would have been easy to file this as unreproducible. The steps you did give were exactly right — the Tab into the preview is what puts focus in the reader when the extraction lands, which is one of the three conditions. The other two are the classic reader and an article whose extracted text runs over 16384 characters, which is why it only ever hit some articles. Please do reopen if you see any stale text after updating.