From cc45db5ef83d0dbbd1f9e7c5f2f4f390318e3966 Mon Sep 17 00:00:00 2001 From: Morax <100508620+fzlzjerry@users.noreply.github.com> Date: Thu, 10 Sep 2026 23:18:09 -0700 Subject: [PATCH] Fix adaptive reading state after marking articles read (#9290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #9288 Changes proposed in this pull request: The mark-as-read links contain the resolved state, so the automatic reading preference becomes an explicit `state=2` filter after the redirect. Once the last unread article is marked read, the page stays empty instead of showing all articles. - Keep the requested state separately from the resolved state used by the action. A redirect state of `0` reapplies the reading preference; explicit filters still survive the redirect as intended by #9007. - Carry that value through the menu, footer and “Load more” link. This matters with oldest-first pagination, where the last footer supplies the menu button's action. - Add request-state and rendered-pagination tests, plus a changelog entry. How to test the feature manually: 1. With both read and unread articles, choose “Show unreads if any, all articles otherwise”. Mark all as read from the footer, then repeat using the top button. The resulting page should show the read articles. 2. Set one article per page and oldest-first sorting. Load the remaining articles before marking all as read; the same fallback should work. 3. Select an explicit unread filter: marking everything read should still leave an empty unread view. Also check a favourite filter with a search term, including “Mark selection unread”; the search and favourite scope should remain intact. Validation: `make NO_DOCKER=1 fix-all` and `make NO_DOCKER=1 test-all` pass with PHP 8.5.9 (719 PHPUnit tests, 1,378 assertions). Browser checks passed against SQLite in Chromium (desktop and mobile viewport) and Firefox. The original footer fails three of the six new rendered-pagination cases. --- CHANGELOG.md | 3 ++ app/Controllers/entryController.php | 2 +- app/Models/Context.php | 5 +++ app/layout/nav_menu.phtml | 1 + app/views/helpers/stream-footer.phtml | 2 ++ tests/app/Models/ContextTest.php | 41 +++++++++++++++++++++++ tests/app/Views/streamFooterTest.php | 47 +++++++++++++++++++++++++++ 7 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 tests/app/Models/ContextTest.php create mode 100644 tests/app/Views/streamFooterTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 51b23df7d..e8174d425 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ See also [the FreshRSS releases](https://github.com/FreshRSS/FreshRSS/releases). ## 2026-XX-XX FreshRSS 1.30.1-dev +* Bug fixes + * Restore the automatic reading view after marking articles as read, while preserving explicit filters [#9288](https://github.com/FreshRSS/FreshRSS/issues/9288) + ## 2026-09-09 FreshRSS 1.30.0 diff --git a/app/Controllers/entryController.php b/app/Controllers/entryController.php index 33f526781..37382a2db 100644 --- a/app/Controllers/entryController.php +++ b/app/Controllers/entryController.php @@ -207,7 +207,7 @@ class FreshRSS_entry_Controller extends FreshRSS_ActionController { if ($search !== '') { $params['search'] = $search; } - $stateParam = Minz_Request::paramInt('state'); + $stateParam = FreshRSS_Context::getStateForRedirect(); if ($stateParam !== 0) { $params['state'] = $stateParam; } diff --git a/app/Models/Context.php b/app/Models/Context.php index 2eec03f21..29a1688a7 100644 --- a/app/Models/Context.php +++ b/app/Models/Context.php @@ -317,6 +317,11 @@ final class FreshRSS_Context { self::$sinceHours = Minz_Request::paramInt('hours'); } + /** Return the requested navigation state, or 0 to reapply the reading preference after the action. */ + public static function getStateForRedirect(): int { + return Minz_Request::paramIntNull('stateForRedirect') ?? Minz_Request::paramInt('state'); + } + /** * Checks whether the $state parameter is consequential, i.e. has any effect * (not zero, and not just including opposite states). diff --git a/app/layout/nav_menu.phtml b/app/layout/nav_menu.phtml index e41ccb0bc..8ed94f768 100644 --- a/app/layout/nav_menu.phtml +++ b/app/layout/nav_menu.phtml @@ -128,6 +128,7 @@ 'idMax' => FreshRSS_Context::$id_max, 'search' => FreshRSS_Context::$search->toString(), 'state' => FreshRSS_Context::$state, + 'stateForRedirect' => FreshRSS_Context::getStateForRedirect(), 'sort' => FreshRSS_Context::$sort, 'order' => FreshRSS_Context::$order, 'from' => Minz_Request::actionName(), diff --git a/app/views/helpers/stream-footer.phtml b/app/views/helpers/stream-footer.phtml index 2b6995196..44bbbd01e 100644 --- a/app/views/helpers/stream-footer.phtml +++ b/app/views/helpers/stream-footer.phtml @@ -9,6 +9,7 @@ $url_next['params']['idMax'] = FreshRSS_Context::$id_max; } $url_next['params']['state'] = (string)FreshRSS_Context::$state; + $url_next['params']['stateForRedirect'] = FreshRSS_Context::getStateForRedirect(); $url_next['params']['ajax'] = '1'; $url_mark_read = [ @@ -20,6 +21,7 @@ 'idMax' => FreshRSS_Context::$id_max, 'search' => FreshRSS_Context::$search->toString(), 'state' => FreshRSS_Context::$state, + 'stateForRedirect' => FreshRSS_Context::getStateForRedirect(), 'sort' => FreshRSS_Context::$sort, 'order' => FreshRSS_Context::$order, 'from' => Minz_Request::paramStringNull('from') ?? Minz_Request::actionName(), diff --git a/tests/app/Models/ContextTest.php b/tests/app/Models/ContextTest.php new file mode 100644 index 000000000..f8e8afe8f --- /dev/null +++ b/tests/app/Models/ContextTest.php @@ -0,0 +1,41 @@ +,int}> */ + public static function provideRedirectStates(): Traversable { + yield 'default preference' => [[], 0]; + yield 'legacy unread filter' => [['state' => '2'], 2]; + yield 'legacy favourite filter' => [['state' => '4'], 4]; + yield 'legacy all articles' => [['state' => '3'], 3]; + yield 'automatic unread view' => [['state' => '2', 'stateForRedirect' => '0'], 0]; + yield 'automatic all view' => [['state' => '3', 'stateForRedirect' => '0'], 0]; + yield 'explicit unread view' => [['state' => '2', 'stateForRedirect' => '2'], 2]; + yield 'explicit favourite view' => [['state' => '4', 'stateForRedirect' => '4'], 4]; + yield 'original filter takes precedence' => [['state' => '2', 'stateForRedirect' => '4'], 4]; + yield 'integer zero remains automatic' => [['state' => 2, 'stateForRedirect' => 0], 0]; + yield 'invalid origin falls back' => [['state' => '2', 'stateForRedirect' => 'invalid'], 2]; + yield 'array origin falls back' => [['state' => '4', 'stateForRedirect' => ['0']], 4]; + } + + /** @param array $params */ + #[DataProvider('provideRedirectStates')] + public static function testStateForRedirect(array $params, int $expected): void { + $original = Minz_Request::params(); + try { + Minz_Request::_params($params); + self::assertSame($expected, FreshRSS_Context::getStateForRedirect()); + self::assertSame($params, Minz_Request::params()); + } finally { + Minz_Request::_params($original); + } + } + + public static function testAutomaticStateSurvivesUrlEncoding(): void { + $url = Minz_Url::display(['params' => ['state' => 2, 'stateForRedirect' => 0]], 'ascii'); + self::assertStringContainsString('state=2&stateForRedirect=0', $url); + } +} diff --git a/tests/app/Views/streamFooterTest.php b/tests/app/Views/streamFooterTest.php new file mode 100644 index 000000000..7a76da5cb --- /dev/null +++ b/tests/app/Views/streamFooterTest.php @@ -0,0 +1,47 @@ +,int,string}> */ + public static function providePaginationStates(): Traversable { + yield 'automatic first page' => [[], 2, '0']; + yield 'explicit unread first page' => [['state' => '2'], 2, '2']; + yield 'explicit favourite first page' => [['state' => '4'], 4, '4']; + yield 'automatic unread next page' => [['state' => '2', 'stateForRedirect' => '0'], 2, '0']; + yield 'automatic all next page' => [['state' => '3', 'stateForRedirect' => '0'], 3, '0']; + yield 'explicit favourite next page' => [['state' => '4', 'stateForRedirect' => '4'], 4, '4']; + } + + /** @param array $request */ + #[DataProvider('providePaginationStates')] + public function testPaginationKeepsTheRequestedState(array $request, int $resolvedState, string $requestedState): void { + FreshRSS_Context::initSystem(); + Minz_Translate::init('en'); + FreshRSS_Context::$state = $resolvedState; + FreshRSS_Context::$continuation_id = '123'; + FreshRSS_Context::$search = new FreshRSS_BooleanSearch('needle'); + Minz_Request::_params($request + ['search' => 'needle']); + + $view = new FreshRSS_View(); + $view->_path('helpers/stream-footer.phtml'); + $html = new DOMDocument(); + self::assertTrue($html->loadHTML($view->renderToString(), LIBXML_NONET)); + $button = $html->getElementById('load_more'); + self::assertInstanceOf(DOMElement::class, $button); + $query = parse_url($button->getAttribute('formaction'), PHP_URL_QUERY); + self::assertIsString($query); + parse_str($query, $params); + self::assertSame((string)$resolvedState, $params['state'] ?? null); + self::assertSame($requestedState, $params['stateForRedirect'] ?? null); + self::assertSame('needle', $params['search'] ?? null); + self::assertSame('123', $params['cid'] ?? null); + self::assertSame('1', $params['ajax'] ?? null); + } +}