Fieldset formwidget fields should be translatable

[mjauvin]

Summary by CodeRabbit

• Bug Fixes
  • Improved nested form field-set initialization by removing an unnecessary nested-state setting.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 10c4ead8-fb76-4d4a-b5d8-7330fe767b05

📥 Commits

Reviewing files that changed from the base of the PR and between af4b34b and d4494c8.

📒 Files selected for processing (1)

• modules/backend/formwidgets/FieldSet.php

💤 Files with no reviewable changes (1)

• modules/backend/formwidgets/FieldSet.php

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

Walkthrough

The nested form widget setup no longer assigns isNested to true during field set initialization.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to d4494

This is a narrowly scoped change to make fieldset formwidget fields translatable, with no actionable merge-blocking risk remaining beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: making fieldset form widget fields translatable.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch make-fieldset-fields-translatable

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[LukeTowers]

@mjauvin this is going to cause issues with code that extends form widgets that checks for isNested to avoid injecting fields on the wrong Form widget instance.

[AIC-BV]

Sorry for the AI dump
A possible fix in the code suggestion box below

@LukeTowers you're right — EasyAudit, Winter.Pages and Winter.TailwindUI all guard on isNested together with a model instanceof / controller check, and a fieldset's inner form shares both the parent's model and controller, so dropping the flag has them inject a duplicate Activity Log tab / Preview tab / brand fields inside the fieldset.

The root issue is that isNested carries two meanings:

1. "this Form widget is a child of another Form widget" — what extension guards use it for;
2. "this form's data scope differs from the model's attributes" — what the docblock describes ("a good indicator to expect that the form model and dataset values will differ"), and what core and Winter.Translate use it for.

For repeater and nestedform both hold. For fieldset only (1) does: it reuses the parent's model and the parent's arrayName, and merges its getSaveData() straight back up, so its fields are model attributes exactly as if they had been declared on the parent form.

Conflating them breaks two things:

• Winter.Translate skips the form (this PR's motivation) — translatable attributes inside a fieldset never get their ml* widget.
• Core's own required-detection is wrong. Form::defineFormField() prefixes the attribute name for nested forms; with the fieldset's inherited arrayName that yields isAttributeRequired('.website_name') for a parent arrayName of Settings — a name that can never match a rule — so required is silently never auto-detected for fieldset fields.

Suggestion

Keep isNested = true so every existing guard behaves exactly as it does today, and add a narrower flag for the second meaning:

--- a/modules/backend/widgets/Form.php
+++ b/modules/backend/widgets/Form.php
@@ -74,6 +74,15 @@ class Form extends WidgetBase
      */
     public $isNested = false;
 
+    /**
+     * @var bool Used to flag that this nested form shares the data scope of its parent
+     * form; ie. its fields resolve against the same model attributes and array name as
+     * if they had been defined on the parent form directly. Only relevant when
+     * $isNested is true. Used by the fieldset form widget, which nests fields visually
+     * without introducing a new data scope.
+     */
+    public $sharesParentScope = false;
+
     //
     // Object properties
     //
@@ -138,6 +147,7 @@ class Form extends WidgetBase
             'context',
             'arrayName',
             'isNested',
+            'sharesParentScope',
         ]);
 
@@ -881,8 +891,9 @@ class Form extends WidgetBase
         if ($field->required === null && $this->model && method_exists($this->model, 'isAttributeRequired')) {
-            // Check nested fields
-            if ($this->isNested) {
+            // Check nested fields, unless the nested form shares its parent's data
+            // scope, in which case the attribute name needs no prefixing
+            if ($this->isNested && !$this->sharesParentScope) {
                 // Get the current attribute level
                 $nameArray = HtmlHelper::nameToArray($this->arrayName);
                 unset($nameArray[0]);
--- a/modules/backend/formwidgets/FieldSet.php
+++ b/modules/backend/formwidgets/FieldSet.php
@@ -54,6 +54,10 @@ class FieldSet extends FormWidgetBase
         // set arrayName from parent form to save fields to the model
         $config->arrayName = $this->getParentForm()->arrayName;
+        // the fields are nested visually only; they resolve against the parent
+        // form's model and array name, exactly as if defined on the parent form
+        $config->isNested = true;
+        $config->sharesParentScope = true;

Winter.Translate's guard then becomes:

// EventRegistry::registerModelTranslation()
$sharesParentScope = property_exists($widget, 'sharesParentScope') && $widget->sharesParentScope;

if (!$model->hasTranslatableAttributes() || ($widget->isNested && !$sharesParentScope)) {
    return;
}

property_exists() keeps that working against older core releases.

Verified

On a settings model with 22 translatable attributes, all of them inside type: fieldset groups:

• with the change: all 22 render as ml* widgets inside the fieldset;
• with Translate's original $widget->isNested guard restored: 0 — confirming the inner form still reports isNested === true, so third-party guards are untouched.

Happy to open the Winter.Translate PR (and a docs note) alongside if you like the direction. Naming is of course open — isInline, inheritsParentScope, whatever reads best.

Alternative

If you'd rather not add core surface at all: have Winter.Translate recurse into type: fieldset field definitions when it processes the parent form's fields. That fixes translation only, and leaves the required-detection bug — and any other consumer reading isNested as "different data scope" — in place.

[LukeTowers]

@AIC-BV seems reasonable enough, feel free to submit a PR.