Consider showing all revisions on the revision overview

Another related problem at would be solved by this: The paging is done in NodeController::getRevisionIds(); that returns 50 revisions at a time. Then in ::revisionOverview(), some of the revisions may be skipped. This can result in an empty results page. I've seen a pager with several pages, but only two revisions that actually appear anywhere.

Is this something we still can/want to/should consider?

I think we should do this.

A language selector would allow for a closer equivalent to the current behaviour for people that need it, but the page just does not properly work as it currently is, so I think we could open a separate issue to add that, and keep things simple here.

Added related/dupe issues for core and Diff. I think the main issue here is the isRevisionTranslationAffected check.

The generic revision UI also has this same check in VersionHistoryController::loadRevisions. Whatever fix we do for Node should apply there too.

Agreed that we should keep this one simple and potentially add a language filter in another issue if needed.

These conditions were added in #2465907: Node revision UI reverts multiple languages when only one language should be reverted → so it will be crucial we don't regress on that bug

Commit https://git.drupalcode.org/project/drupal/-/commit/7940793ae0bbfdb64fb5e...

After debugging this more it's clear that the current code is definitely masking some pretty weird issues.

E.g set up the following:
1. Install standard
2. Install content_translation module
3. Add another language (e.g Dutch)
4. Edit Basic page content type and mark it translatable
5. Create a node, then translate it to the new language, adding a revision log of "create LANGUAGE revision".

At this point this will create 2 new revisions - one for English and one for Dutch. The English one will be marked with revision_translation_affected = NULL
The node_revision table will have 2 records, but node_field_revision will have 3. 2 of those will be for vid 2, one for each language. Vid 2 for english will have values copied from vid 1 (i.e created/changed dates).
Since the revision_log field is not translateable, the node_revision table will have "create LANGUAGE revision" for vid 2, which is the default revision for english.

6. Go to node/1/revisions (i.e the English revisions page). You'll see 1 revision marked as current, but this is actually revision 1. You won't see a revision log message, which IMO is incorrect. The reason this is incorrectly shown as the current revision is because of this code:

        // We treat also the latest translation-affecting revision as current
        // revision, if it was the default revision, as its values for the
        // current language will be the same of the current default revision in
        // this case.
        $is_current_revision = $revision->isDefaultRevision() || (!$current_revision_displayed && $revision->wasDefaultRevision());

This is green now.

Want to get a review on the code changes before going ahead with a full IS update + CR.

Can the summary be updated with what’s being proposed? Do we want a page with all revisions loaded? What if it’s 1000s of revisions

@smustgrave did you see #29?

Do we want a page with all revisions loaded

No, we still paginate revisions.

Yea but without a clear summary hard to review the code as it wasn’t clear what the we being accomplished

Any recommended way for testing this one?

@smustgrave it's a bit tricky because you have to generate revisions that are marked as non translation affecting.

The easiest way to do that is programatically generate some revisions similar to the tests.

Ran test-only feature

1) Drupal\Tests\node\Functional\NodeRevisionsUiTest::testNodeRevisionsTabWithDefaultRevision
Behat\Mink\Exception\ExpectationException: 2 elements matching css ".node-revision-table tbody tr" found on the page, but should be 5.
/builds/issue/drupal-2722307/vendor/behat/mink/src/WebAssert.php:888
/builds/issue/drupal-2722307/vendor/behat/mink/src/WebAssert.php:441
/builds/issue/drupal-2722307/core/modules/node/tests/src/Functional/NodeRevisionsUiTest.php:176
FAILURES!
Tests: 4, Assertions: 40, Failures: 1, PHPUnit Deprecations: 10.
Exiting with EXIT_CODE=1

Tried to do the same as the tests manually but ended up borking it so going to lean on the test coverage here.

But overall believe this one is good and don't see any open items for it. Going to mark.