Undo, redo and history are not language-aware

Code review

OK, I guess. There are some surprising stuff, especially the history_step integer base field in Instance entity, but I have nothing else to propose. Just smelling something I hope we will not regret later.

Feature review

1. Tested with 2 languages (EN & DE) on a Page Layout.
2. 10:44: I add a textfield in EN with "English" value. A step is added to history.
3. 10:45: I translate this textfield in DE with "Deutsch" value. A step is added to history.
4. I go back to English: I don't see the 10:45 step in history
5. 10:46: I update Englsih value with "EN 2". A step is added to history.
6. I go back to German: I don't see the 10:46 step in history
7. I undo, it adds a "I go back to English: I don't see the 10:45 step in history" step in 10:48

This "undo" step, works as described in comment #4, but is very surprising and is subverting the concept of undo/redo.

Maybe it is the cost to pay to have something working with tarnslations, but I have noticed than we have the same "undo" step when doing undo in a non translated display.

Is this the right model? The alternative was "a step belongs to the language it was made in". I dropped it: undoing a text edit could then silently revert a structural change another language made in between.

I don't know yet, I am still confused about such a choice. I may need time to process.

Jean, are you the one asking the "For review" questions or is it Claude? If it is the latter, what would be your answers/positions?

Review in progress...

Check the different revisions logs logic (as visible in the island dropdown):

Careful with publishing, it seems it changes what is shown.

There are still "undo" steps...

Is this review the opportunity to restore the "Language" col which has been removed from #3555110: Symmetric translation?

Discussed with Jean during last weekly: if the strange current beahviour is the "normal" Druapl behaviour when an entity is both revisionable and translatable, let's keep like that for now and postpone this ticket post 1.0.0.

If we introduced this strange behaviour, let's address it ASAP.

Opening a new branch to test the "normal" Drupal Core behavior for translatable revisionable entities.

In Drupal 11, revisions belong to the entity, not to individual translations. There’s no separate revision ID per language. All translations share one revision history. So, saving any translation with “Create new revision” enabled creates a new revision for the whole entity.

However, each translation row in a revision has a revision_translation_affected flag. It’s TRUE only for the languages whose values actually changed in that save, and this is introducing interesting stuff in Revision UI:

• The Revisions tab uses this flag to filter the list per language. When you view revisions for the French translation, you only see revisions where French was affected, even though every revision technically contains French
• The revision UI has a “Revert translation” action. A checkbox, “Revert content shared among translations”, controls whether untranslatable fields are reverted too. It’s only available on the default translation, or when untranslatable fields are editable there.

Let's check how it works and how we can leverage this.

Tested with both:

Taxonomy term, Tag vocabulary

• Translatable fields: "Name" and "Changed"
• Non translatable fields: "Description", "Published" and "URL Alias"

I have also hooked VersionHistoryController to show if a translation is the only one "affected" or not:

It is a temporary generic alteration for now, but we may need than in Display Builder Revision Log.

Let's check revert now:

So, Core revisions are weird, indeed:

• Changed field is not updated while being a translatable field
• Some revisions made by authenticated user are attributed to Anonymous
• Reverting to a non affected revision create an affected revision

But there are interesting stuff to explore.

Display Builder instance

Adding this to check directly in Drupal UI (without our specific stuff):

links: [
  'version-history' => '/admin/structure/display-builder/instances/{display_builder_instance}/revisions',
],

Our revisions management is very different from Core:

Here is the proposal : https://git.drupalcode.org/project/display_builder/-/merge_requests/488

Finally, I am proposing to assume we have 2 big differences with Drupal Core revision management (while being closer to the Core Content Moderation" module):

• because of the storage strategy: undo/redo revert without creating new revisions
• because of the UX (undo/redo buttons instead of a "revert" button on each revision row): we can revert from revision of any language to revision of any language

Assuming this, we just need 2 changes to fix our revisions management:

• fix a bug where a target revision become non affected
• make this obvious in UI by adding a "Language" col in HistoryButton

If you are OK with the current MR to review, I can add an extra commit to display the Language col only when there are translations, in order to not make the UI too busy.

beta9 has been released. Changing the tag.

Rebased. Open to anybody to review.

✅ Tested and works — applied MR !488, rebuilt caches, then on a fresh page layout edited a Textfield in EN, FR, EN again and clicked undo from /fr/: the history now shows a Language column, and the FR-only step keeps revision_translation_affected NULL for EN after undo, where without the fix it flipped to 1. The flag bug is fixed; undo from FR still reverts the EN edit, as proposed with MR !488.

Prerequisites

• Languages en + fr, content_translation and config_translation enabled; a new page layout duplicated from test_empty.
• Flags checked with SELECT revision, langcode, revision_translation_affected FROM display_builder_instance_field_revision.

Actual results

Notes

• The issue summary still proposes per-language undo with a history_step field; MR !488 takes the narrower route from its proposal comment, so the IS needs an update before commit.

Code review

• MR !488 at 3579098, pipeline green; testUndoAndRedoKeepTranslationAffectedFlags() covers the flag regression.

The issue summary still proposes per-language undo with a history_step field; MR !488 takes the narrower route from its proposal comment, so the IS needs an update before commit.

Indeed. The proposal diverges from initial issue summary in purpose. Not having history_step is a benefit. Flowing naturally in the revision log is an other.