Mform 9 :: repeater2 und neue widgets - #396
Merged
Merged
Conversation
skerbis
requested review from
Copilot,
eaCe and
joachimdoerr
and removed request for
Copilot
May 5, 2026 22:56
Contributor
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Moin WalkthroughGroßer Release‑Commit: Version 9.0.0 bringt einen Vanilla‑JS Flex‑Repeater (verschachtelt), bedingte Feldsicht (Conditionals), List/Medialist/Linklist + Multi‑CustomLink Widgets, Template‑Registry, neue YForm‑Value‑Types, umfangreiche Frontend/CSS‑Implementierungen, Parser/Renderer‑Änderungen sowie zahlreiche Doku‑ und Demo‑Ergänzungen. ChangesMForm v9 — Haupt‑DAG
Sequence Diagram (Flex‑Repeater high level)sequenceDiagram
participant User as Benutzer
participant Browser as Browser
participant RepeaterJS as Flex‑Repeater (assets/js/flex-repeater.js)
participant Editor as TinyMCE/CKEditor
participant Server as PHP/Parser/Renderer
participant DB as Datenbank
User->>Browser: öffnet Modul‑Seite
Browser->>Server: GET Modul (Renderer)
Server->>Server: MFormParser -> MFormFlexRepeaterRenderer.renderTemplate()
Server-->>Browser: HTML mit data-mfr-* Attributen
Browser->>RepeaterJS: mfrInit(container)
RepeaterJS->>Editor: init/destroy/reinit Hooks (bei Items)
User->>RepeaterJS: Item hinzufügen/verschieben/editieren
RepeaterJS->>RepeaterJS: update JSON hidden input
Browser->>Server: POST Form
Server->>Server: MFormRepeaterHelper::decode() -> filter __disabled
Server->>DB: Persistieren
Sequence Diagram (List‑Widget / Popup flow)sequenceDiagram
participant User as Benutzer
participant UI as List‑Widget UI
participant JS as list-widget.js
participant Popup as Medien/Link Popup
participant Hidden as Hidden Input
User->>UI: Klick "Hinzufügen"
UI->>Popup: writeREXMedialist / writeREXLinklist
Popup->>User: Auswahl treffen
Popup->>JS: global hook ruft registered callback
JS->>JS: mformListBuildOptionsFromHidden()
JS->>UI: mformListRender() (Thumbnail + data-preview)
JS->>Hidden: mformListWriteHidden() (CSV/Komma‑Liste)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lang/fr_fr.lang (1)
68-82:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFehlender Übersetzungsschlüssel
yform_values_custom_link_anchor.Moin –
lib/yform/value/custom_link.php(Line 40) referenziertrex_i18n::msg('yform_values_custom_link_anchor'), aber dieser Schlüssel fehlt infr_fr.lang. Das führt im französischen Backend zu einem leeren oder rohen Schlüsselnamen als Label für die neue Anchor-Checkbox.➕ Vorgeschlagene Ergänzung
yform_values_custom_link_ylink = Widget de lien YForm (label_text::yform_table::column_display,label_text2::yform_table_2::column_display_2) +yform_values_custom_link_anchor = Bouton d'ancre🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lang/fr_fr.lang` around lines 68 - 82, Add the missing French translation for the i18n key referenced by lib/yform/value/custom_link.php: yform_values_custom_link_anchor is used as the label for the anchor checkbox but does not exist in lang/fr_fr.lang; add an appropriate French string (e.g. "Bouton d'ancre" or "Ancre") as yform_values_custom_link_anchor in the fr_fr.lang file so the backend shows a proper label instead of the raw key.lib/MForm/Parser/MFormParser.php (1)
998-1009:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMoin – die neue Widget-ID-Bildung kann hier deterministisch kollidieren.
Vor dem finalen
crc32()werden die Segmente separatorlos mitimplode('', $varId)zusammengezogen. Damit kollabieren z. B.['1', '23']und['12', '3']beide zu123und erzeugen dieselbe Widget-ID. Bei Link-/Media-Widgets reicht so eine Kollision, damit Popup-Aktionen das falsche Feld treffen.Diff-Vorschlag
- return (string) abs(crc32(implode('', $varId))); + return (string) abs(crc32(implode('|', array_map('strval', $varId))));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/MForm/Parser/MFormParser.php` around lines 998 - 1009, The current getWidgetId(MFormItem $item) builds the final crc32 over implode('', $varId) which causes collisions like ['1','23'] vs ['12','3']; change the concatenation to preserve segment boundaries (e.g. join with a stable delimiter or encode the array) before computing crc32 so segments can't merge—update getWidgetId to use a delimiter-aware join (or json_encode($varId)) when forming the string fed to abs(crc32(...))) and keep references to MFormItem::getVarId()/setVarId and getWidgetId in your change.
🟡 Minor comments (12)
assets/mform.js-136-157 (1)
136-157:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFehlendes Quellfeld blendet Target nicht aus – möglicherweise unerwünschtes Default-Verhalten.
Wenn
findSourceFields(source)keinen Treffer im aktuellenmform-Scope liefert (z. B. weil die Quelle außerhalb des permblock:changeübergebenen Subbaums liegt oder der DOM-Aufbau ein anderer Container ist), wird das Target stumm viatarget.show()immer eingeblendet. Beiaction: 'hide'ist das aber das Gegenteil der Intention. Empfehlung: bei fehlender Quelle das Target gemäßaction-Default behandeln (z. B. nicht touchen) oder zumindest in der Konsole loggen, damit Konfigurationsfehler auffallen.🛡️ Vorschlag
if (!fields.length) { - target.show(); + // Quelle nicht auflösbar: Default-State je nach action behalten + if (window.console && console.warn) { + console.warn('[mform] conditional source not found:', source); + } + target.show(); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/mform.js` around lines 136 - 157, In applyConditional, when findSourceFields(source) returns no fields the current logic always calls target.show(), which breaks configurations that expect action:'hide'; change the no-fields branch in applyConditional (the block using findSourceFields and variable action) to handle missing source according to action: if action === 'hide' then mark/hide the target (e.g. add 'mform-conditional-hidden' and call target.hide()), otherwise leave it shown (or unchanged); also emit a console.warn or console.debug mentioning the missing source and the affected target to aid debugging (reference function applyConditional, findSourceFields, variable action and data attribute mform-conditional-action).lib/Widget/var_custom_medialist.php-79-95 (1)
79-95:⚠️ Potential issue | 🟡 Minor | ⚡ Quick wini18n-Keys für View-Switch in fr_fr.lang ergänzen.
Moin, der View-Switch nutzt
mform_list_widget_view_listundmform_list_widget_view_grid. Die Keys sind in de_de.lang und en_gb.lang vorhanden, fehlen aber in fr_fr.lang. Das führt dazu, dass französischsprachige Nutzer die nackten Key-Namen statt der Übersetzung im Tooltip sehen.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/Widget/var_custom_medialist.php` around lines 79 - 95, The French language file is missing the i18n keys used by the view switch (mform_list_widget_view_list and mform_list_widget_view_grid) referenced in var_custom_medialist.php; add these two keys to fr_fr.lang with appropriate French translations (e.g., "Liste" and "Grille" or your preferred phrasing) so the tooltips rendered by the viewButton show translated labels instead of raw key names; ensure the keys exactly match mform_list_widget_view_list and mform_list_widget_view_grid and follow the same file format/encoding as the other entries in fr_fr.lang.pages/module/extended/conditional_fields_builder/output.inc-3-3 (1)
3-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin – inkonsistentes Platzhalter-Format und fehlende Dekodierung.
Zwei Punkte fallen auf:
Platzhalter-Format: Alle anderen Output-Templates verwenden
REX_VALUE[id=1](expliziteid=-Syntax), dieses File nutztREX_VALUE[1](Kurzform). Das funktioniert in REDAXO zwar auch, erzeugt aber Inkonsistenz – besonders wenn andere Entwickler dieses Template als Vorlage nehmen.Keine Dekodierung: Wenn
addConditionalFieldsetAreastrukturierte Daten (z. B. JSON) speichert, liefertdump('REX_VALUE[1]')nur den rohen gespeicherten String. Falls die gespeicherten Werte mitMFormRepeaterHelper::decode()verarbeitet werden sollen, wäre das hier auszutauschen.🔧 Vorgeschlagene Anpassung (analog den anderen Output-Templates)
-dump('REX_VALUE[1]'); +dump(\FriendsOfRedaxo\MForm\Repeater\MFormRepeaterHelper::decode('REX_VALUE[id=1]'));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/module/extended/conditional_fields_builder/output.inc` at line 3, Ersetze die inkonsistente Kurz-Placeholder-Nutzung und das rohe Dumpen: tausche dump('REX_VALUE[1]') gegen das explizite Placeholder-Format REX_VALUE[id=1] und übergebe den Platzhalterinhalt vor dem Dump an MFormRepeaterHelper::decode() (oder die passende Decode-Methode), sodass addConditionalFieldsetArea/Output das decodierte Struktur-Array statt des rohen Strings erhält; referenziere hierbei die vorhandene dump-Funktion, den Platzhalter REX_VALUE[id=1] und MFormRepeaterHelper::decode.docs/09_templates.md-13-15 (1)
13-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin, Widerspruch bei Registry-Verantwortung in der Doku.
Oben steht, dass MForm die Registry mitliefert, in der Beispielstruktur wird aber zusätzlich
lib/MFormTemplate/TemplateRegistry.phpim Projekt empfohlen. Das wirkt widersprüchlich und führt leicht zu Doppel-Implementierungen. Bitte die Struktur-Liste entsprechend bereinigen oder den Sonderfall explizit erklären.Also applies to: 21-26
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/09_templates.md` around lines 13 - 15, Die Doku widerspricht sich zur Registry-Verantwortung: entweder MForm liefert die Registry intern oder das Projekt muss eine eigene Registry bereitstellen (z.B. TemplateRegistry), also korrigiere den Text so, dass es eine einzige klare Verantwortung gibt — entweder entferne die Aussage "MForm liefert die Registry selbst mit" und beschreibe, wie Projekte eine eigene Registry (TemplateRegistry) registrieren und wie MForm::fromTemplate() / ->applyTemplate() diese Registry nutzen, oder belasse die Aussage und ergänze explizit den Sonderfall, dass Projekte nur Erweiterungen/Registrierungen (key + Klassenname) vornehmen dürfen und keine eigene Registry-Klasse erstellen dürfen; referenziere dabei die Symbole MForm::fromTemplate, ->applyTemplate und TemplateRegistry, damit Leser wissen, welches Verhalten erwartet wird.pages/module/repeater/tinymce_nested_repeater/output.inc-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin –
REX_VALUE[1]in<h1>ohnerex_escape()Das REDAXO-Token wird durch den gespeicherten Rohwert ersetzt. Enthält dieser HTML-Markup oder ein
<script>-Tag, wird es ungefiltert gerendert.🛡️ Vorgeschlagene Korrektur
-<h1>REX_VALUE[1]</h1> +<h1><?= rex_escape('REX_VALUE[1]') ?></h1>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/module/repeater/tinymce_nested_repeater/output.inc` at line 5, REX_VALUE[1] is being output raw inside an <h1>, which can render untrusted HTML/JS; update the template to escape the token output (e.g., replace REX_VALUE[1] with rex_escape(REX_VALUE[1]) or use htmlspecialchars(REX_VALUE[1], ENT_QUOTES, 'UTF-8')) so the heading prints safe text; locate the literal "REX_VALUE[1]" in the output template and wrap it with the chosen escaping helper (rex_escape or htmlspecialchars) to ensure any HTML or <script> tags are neutralized.assets/css/list-widget.css-111-111 (1)
111-111:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin –
word-break: break-wordist veraltet;overflow-wrap: break-wordverwenden.
word-break: break-wordist alsdeprecatedmarkiert. Die korrekte Eigenschaft dafür istoverflow-wrap, konkretoverflow-wrap: break-word– Browser unterstützenword-break: break-wordweiterhin aus Rückwärtskompatibilität, aber Linting-Tools und Validatoren werden es als Fehler ankreiden.🐛 Vorgeschlagener Fix
-.mform-list-widget.mform-list-widget-medialist.is-grid-view .mform-list-items li { white-space: normal; min-height: 48px; display: flex; flex-direction: column; align-items: flex-start; gap: .45rem; - word-break: break-word; + overflow-wrap: break-word; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/css/list-widget.css` at line 111, Replace the deprecated CSS property "word-break: break-word;" with "overflow-wrap: break-word;" in the rule that currently contains "word-break: break-word;" so the styles use the modern, standards-compliant property; update the declaration (remove or replace the old "word-break: break-word;" line) and keep any other existing declarations in that selector intact.pages/module/repeater/full_feature_lab/output.inc-89-91 (1)
89-91:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin –
$isDisabled-Check ist toter Code:MFormRepeaterHelper::decode()entfernt__disabledbereits.Laut Eigenkommentar (Zeile 31) filtert
decode()deaktivierte Items heraus und entfernt den internen Schlüssel aus den verbleibenden Einträgen (vgl.prepareItemsForOutput:unset($item[self::DISABLED_KEY])). Damit ist$row['__disabled']nachdecode()niemals gesetzt –$isDisabledist immerfalse, daspanel-default-Styling und das<em>(deaktiviert)</em>-Label erscheinen nie.Wenn diese Debug-Ausgabe absichtlich alle Items (inkl. Offline-Items) mit Status anzeigen soll, ist
decode()das falsche Mittel – dann wäre rohes JSON-Decoding + manuelle Filterung nötig. Sollen nur aktive Items ausgegeben werden (aktuelles Verhalten), ist der$isDisabled-Block zu entfernen.🐛 Vorgeschlagener Fix (nur aktive Items, ohne totem Code)
foreach ($rows as $i => $row) { - $isDisabled = !empty($row['__disabled']); - echo '<div class="panel ' . ($isDisabled ? 'panel-default' : 'panel-primary') . '" style="margin-bottom:.5rem">'; - echo '<div class="panel-heading">Zeile ' . ($i + 1) . ($isDisabled ? ' <em>(deaktiviert)</em>' : '') . '</div>'; + echo '<div class="panel panel-primary" style="margin-bottom:.5rem">'; + echo '<div class="panel-heading">Zeile ' . ($i + 1) . '</div>';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/module/repeater/full_feature_lab/output.inc` around lines 89 - 91, The check for $isDisabled is dead because MFormRepeaterHelper::decode() (and its prepareItemsForOutput/unset(self::DISABLED_KEY)) removes the __disabled key; remove the dead branch: drop the $isDisabled variable and the conditional class/label logic in pages/module/repeater/full_feature_lab/output.inc so the panel always renders as the active state (or, if you actually need to show disabled items, replace decode() usage with raw JSON decoding and manual inspection of __disabled before unset); locate the code around the echo lines that build the panel class and heading to apply the change (references: $isDisabled, MFormRepeaterHelper::decode(), prepareItemsForOutput, DISABLED_KEY).assets/js/customlink.js-421-425 (1)
421-425:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winToter Mousedown-Handler kann entfernt werden.
Der Handler bindet nur einen Listener mit auskommentiertem Body („skip for now“). Er erzeugt zusätzliche Bindungen, die nichts tun. Wenn Sortable/Drag wirklich später nachgezogen wird, kann das in einem dedizierten PR ergänzt werden – aktuell ist es Dead Code.
🧹 Vorschlag
- // Sortable via move-up/move-down on drag handle click (simple swap) - multiWidget.on('mousedown.clmulti', '.mform-cl-multi-handle', function (e) { - // Only drag-to-reorder is complex; skip for now – dragging not wired without dragula/sortable - // A future improvement can add sortable library here - });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/js/customlink.js` around lines 421 - 425, Entferne den toten mousedown-Handler: lösche die an multiWidget gebundene Listener-Registration multiWidget.on('mousedown.clmulti', '.mform-cl-multi-handle', ...), da der Callback leer/auskommentiert ist und nur unnötige Bindungen erzeugt; wenn später Drag/Sortable wirklich hinzugefügt wird, implementiere das in einem eigenen PR und verwende dort eine dedizierte Funktion oder Bibliothek statt dieses Platzhalers.README.de.md-14-25 (1)
14-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInkonsistente Umlaut-Schreibweise im neuen Version-9-Abschnitt.
Im Rest des README werden Umlaute durchgängig benutzt (z. B. „über", „Übernahme", „Hinweistext"), in den neuen Bullets aber teilweise transliteriert:
gefuellter,gruen,unveraendert. Da das nutzerseitige Doku-Text ist, lohnt sich eine kleine Vereinheitlichung.✏️ Vorschlag
- - Der Status ist im Header sofort sichtbar: gefuellter Punkt (gruen = aktiv, rot = offline) + - Der Status ist im Header sofort sichtbar: gefüllter Punkt (grün = aktiv, rot = offline) @@ -- Neue API `addCustomLinkMultipleField(...)` – Repeater-basiertes Multi-Link-Feld; Single-Format bleibt unveraendert +- Neue API `addCustomLinkMultipleField(...)` – Repeater-basiertes Multi-Link-Feld; Single-Format bleibt unverändert🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.de.md` around lines 14 - 25, In the German README (Version‑9 bullet list) replace transliterated Umlaut spellings with proper Umlaut characters (e.g. "gefuellter"→"gefüllter", "gruen"→"grün", "unveraendert"→"unverändert" and similar occurrences) so the user-facing docs are consistent; update the bullets that mention addRepeaterElement(), addCustomLinkMultipleField(...), addConditionalFieldsetArea(...), MFormRepeaterHelper::decode(), and the value-types custom_link / custom_link_multi to use the corrected Umlaut forms.lib/yform/value/custom_link_multi.php-50-65 (1)
50-65:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin,
getListValue()solltenulldefensiv behandeln.
'' === $params['value']greift nur bei Leerstring. Da die DB-Spalte vom Typtextist, kann sieNULLenthalten. Wirdnullweitergeleitet, gelangt es inhtml_entity_decode($params['value'], …)auf Zeile 55. Das erzeugt ab PHP 8.1 eine Deprecation-Warnung und wird ab PHP 9 zum Fatal Error. Auch der Rückgabepfadrex_escape($params['value'])auf Zeile 58 würde dannnullverarbeiten.🛡️ Vorschlag
public static function getListValue($params) { - if ('' === $params['value']) { + $value = $params['value'] ?? ''; + if ('' === $value) { return '-'; } - $rawValue = html_entity_decode($params['value'], ENT_QUOTES | ENT_HTML5, 'UTF-8'); + $rawValue = html_entity_decode((string) $value, ENT_QUOTES | ENT_HTML5, 'UTF-8'); $links = json_decode($rawValue, true); if (!is_array($links)) { - return rex_escape($params['value']); + return rex_escape((string) $value); } $out = []; foreach ($links as $link) { $out[] = rex_var_custom_link::getCustomLinkText((string) $link); } return implode(', ', $out); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/yform/value/custom_link_multi.php` around lines 50 - 65, In getListValue, defend against null $params['value'] before calling html_entity_decode/rex_escape: treat null the same as an empty string and return '-' (or coerce to ''), e.g. read the incoming value with a null-coalescing check ($value = $params['value'] ?? '') or an isset check, use $value for html_entity_decode and rex_escape, and ensure rex_var_custom_link::getCustomLinkText() always receives a string so no null is passed into html_entity_decode or rex_escape.assets/js/imglist.js-138-146 (1)
138-146:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin, hardkodierter
/media/-Pfad in dynamisch hinzugefügten Bildern bricht bei REDAXO-Installationen in Unterverzeichnissen.In der Funktion
imglist_add_img_by_last_list_item()wird für SVG/Video-Dateien die Quelle direkt auf/media/<file>gesetzt (Zeile 141). Liegt REDAXO unterhttps://example.com/redaxo-site/, ist der korrekte Pfad aber/redaxo-site/media/....In der PHP-Datei
lib/Widget/var_imglist.phpwird das korrekt mitrex_url::media($file)gelöst (Zeile 81). Das gleiche Widget (var_custom_medialist.php) zeigt die Lösung: Die korrekte Media-URL muss vom Backend viadata-preview-base-Attribut an das JavaScript übergeben werden (Zeile 73-75).Bitte den Media-Base-URL vom PHP-Widget an das JavaScript übergeben statt absoluter Pfade zu verwenden.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/js/imglist.js` around lines 138 - 146, The code currently hardcodes '/media/' for SVG/video in imglist_add_img_by_last_list_item; instead read the media base URL provided by the backend (use the data-preview-base attribute that the PHP widget should emit, e.g. on the widget root or the media list element) and build the source as previewBase + file (or previewBase + encodedFile) rather than '/media/<file>'; update the logic that sets url and source in imglist_add_img_by_last_list_item to derive url = previewBase (fallback to existing index.php path if dataset missing) and then compute source = isVideo ? (url + file) : (url + encodedFile) so installs in subdirectories use the correct base.assets/js/list-widget.js-377-390 (1)
377-390:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin –
beforeunloadals sekundäres Fallback zu schwach.Die
beforeunload-Event ist inmformListBindPopupSyncnur ein sekundärer Fallback; die primäre Kommunikation läuft über die gepatchtenwriteREXLinklist()/writeREXMedialist()-Funktionen, die vonmformListInstallPopupBridge()eingerichtet werden. Das funktioniert für normale Fälle.Allerdings wird
beforeunloadnicht zuverlässig feuern, wenn der Popup viawindow.close()schließt, ohne Navigation auszulösen – etwa wenn der Benutzer das Fenster abbricht, ohne einen Eintrag zu wählen. In diesem Fall wird der Callback nie aufgerufen. Robuster wäre ein Fallback-Poll aufpopup.closedmitsetInterval, das den Callback auch bei ungeplanten Schließungen auslöst.♻️ Vorschlag (Skizze)
function mformListBindPopupSync(popup, callback) { if (!popup || typeof callback !== 'function') { callback(); return; } try { popup.addEventListener('beforeunload', function () { window.setTimeout(callback, 40); }); } catch (e) { window.setTimeout(callback, 80); } + + // Fallback: pollen, falls beforeunload nicht zuverlässig feuert (z. B. window.close()) + const timer = window.setInterval(function () { + try { + if (popup.closed) { + window.clearInterval(timer); + window.setTimeout(callback, 40); + } + } catch (e) { + window.clearInterval(timer); + } + }, 250); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/js/list-widget.js` around lines 377 - 390, mformListBindPopupSync currently relies on the beforeunload event as a fallback which can miss cases where a popup is closed via window.close(); update the fallback to start a short setInterval poll that checks popup.closed and calls the callback (once) when true and then clears the interval; keep the existing beforeunload handler but ensure both paths guard so the callback only runs once (use a local called flag or clear the interval before invoking callback), and clear the interval/handlers if popup becomes null or an error occurs; reference the mformListBindPopupSync and mformListInstallPopupBridge interactions to ensure compatibility with the patched writeREXLinklist()/writeREXMedialist() bridge.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/css/imglist.css`:
- Around line 31-38: Replace the hard-coded German empty-state string in the CSS
selector .rex-js-widget-imglist.is-empty ul.thumbnail-list::before (the content:
'Noch keine Medien ausgewaehlt'; line) with content: attr(data-empty); and
set/update the data-empty attribute from the initializer in assets/js/imglist.js
(where the imglist widget is initialized—e.g. the init or constructor function
that builds/marks .rex-js-widget-imglist instances) to the localized string from
the lang file so the CSS reads the i18n text via data-empty.
In `@assets/css/mform.css`:
- Around line 779-781: Am Ende der CSS-Datei befindet sich eine überflüssige
schließende Klammer '}' ohne passendes öffnendes Block-Statement; behebe das,
indem du die orphaned '}' am Dateiende entfernst, so dass der vorhandene
`@media-Block` und die nachfolgenden Top-Level-Selektoren korrekt geschlossen
bleiben und keine CSS-Syntaxfehler mehr erzeugt.
In `@assets/js/customlink.js`:
- Around line 417-419: Die aktuelle Bindung verwendet
multiWidget.closest('form').off('submit.clmulti').on('submit.clmulti', ...) und
überschreibt dadurch bereits gebundene Submit-Handler anderer
addCustomLinkMultipleField-Instanzen im selben Formular; stattdessen binde den
submit.clmulti-Handler einmal pro Formular (z.B. prüfen ob bereits gebunden ist)
und im Handler nicht nur das gerade initialisierte multiWidget ansprechen,
sondern alle relevanten Widgets im Formular (z.B. per form.find(...) oder einer
gemeinsamen Klasse/Selector für die Multi-Widgets) durchlaufen und für jedes
customLinkMultiSerialize aufrufen; stelle außerdem sicher, dass der bestehende
change-Handler auf den Hidden-Inputs unverändert bleibt und der Submit-Handler
nur als Backup fungiert.
In `@assets/js/flex-repeater.js`:
- Around line 249-257: The checkbox branch currently sets field.checked =
!!(value) which treats string defaults like '0', 'false' or 'off' as truthy;
change it to use the same normalization as the existing helper by replacing the
truthiness check with the inverse of the `_isDisabledValue(value)` logic (e.g.,
set field.checked = !_isDisabledValue(value)) so stored string-false values are
treated as unchecked; update the checkbox branch inside the tag === 'input'
conditional that examines field.type === 'checkbox' and ensure
`_isDisabledValue` is imported/available or inline the same normalization if
necessary.
- Around line 1372-1409: The top-level debug calls currently force logging
(mfrLog(true, ...)) causing full form objects/hidden JSON to leak; update
syncRepeatersInForm and the two document event listeners to call mfrLog with the
global debug flag instead (e.g., mfrLog(isDebugEnabled(), ...)) and avoid
passing raw DOM form objects into logs—log only safe metadata (form.action,
form.method, container counts) rather than the form element itself; keep
existing isContainerDebugEnabled(container) for per-container logs inside
syncRepeatersInForm.
In `@boot.php`:
- Around line 32-33: Die JS-Ladefolge ist falsch: flex-repeater.js erwartet die
globale Funktion initMFormElements, die erst in mform.js definiert wird, so dass
flex-repeater.js jetzt vorzeitig ausgeführt wird und einen Fehler wirft; fix:
tausche die beiden rex_view::addJsFile-Aufrufe so dass
rex_view::addJsFile($this->getAssetsUrl('mform.js')) ausgeführt wird bevor
rex_view::addJsFile($this->getAssetsUrl('js/flex-repeater.js')), damit mform.js
(initMFormElements) geladen und definiert ist bevor flex-repeater.js läuft.
In `@lib/MForm/FlexRepeater/MFormFlexRepeaterRenderer.php`:
- Around line 127-155: The Flex-Repeater renderer in MFormFlexRepeaterRenderer
is treating widget types ('link', 'custom-link', 'media', 'imagelist',
'custom-link-multi') as plain text inputs or falling back to '' so those widgets
break in repeater items; update the switch handling in the renderer to either
call the real widget renderers (use renderListWidget for list types, and the
same widget output used elsewhere for 'link', 'custom-link', 'media',
'imagelist' so they produce full widget markup) or explicitly mark these cases
as unsupported by returning a clear unsupported placeholder via wrapFormGroup;
change the case branches for 'link', 'custom-link', 'media', 'imagelist',
'imagelist'/'medialist' and add handling for 'custom-link-multi' to use
renderListWidget or the shared widget rendering function instead of a simple
<input> or default '' so repeater items match regular widget behaviour.
In `@lib/Widget/var_custom_link.php`:
- Around line 191-195: The anchor was made enabled by default by changing the
condition around $anchorData; revert or tighten it so anchors are only enabled
when the caller explicitly requests it: restore the previous check (use
isset($args['anchor']) && 0 != $args['anchor']) or require the explicit value
'enable' (e.g., $args['anchor'] === 'enable') when setting $anchorData, so
getWidget() callers that omit anchor keep the old behavior; ensure this change
aligns with addCustomLinkField() and the MForm template
(value.custom_link.tpl.php) expectations.
In `@lib/Widget/var_custom_linklist.php`:
- Around line 44-61: Replace the anchor-based toolbar buttons in
var_custom_linklist.php with real button elements and make the disabled state
native and visible: set $disabledAttr (currently $disabled) to ' disabled' and
also add aria-disabled="true" and a CSS class for visual disabled state, and
output <button type="button" class="btn btn-popup mform-list-btn"
data-action="..." title="..."' . $disabledAttr . '
aria-disabled="true">…</button> instead of <a> so keyboard users cannot activate
disabled controls; then update list-widget.js where it currently checks if
($(this).is('[disabled], .disabled')) { return false; } to also handle keydown
for Enter/Space and to test element.disabled or aria-disabled to block
activation from keyboard and mouse consistently.
In `@lib/yform/value/custom_link.php`:
- Line 40: The translation key yform_values_custom_link_anchor referenced by the
'anchor' field in custom_link.php is missing from fr_fr.lang, pt_br.lang,
es_es.lang and sv_se.lang; add the same translation entries as present in
de_de.lang and en_gb.lang into those four language files (using the exact key
yform_values_custom_link_anchor and appropriate localized text) so the Anchor
checkbox label displays correctly.
In `@pages/module/extended/conditional_fields/output.inc`:
- Around line 52-65: The current video branch embeds whatever $videoUrl was
passed because preg_replace returns the original input on no-match; update the
case 'video' handling to verify that preg_replace actually produced a safe
YouTube embed before echoing the iframe: after computing $embedUrl with
preg_replace, check that $embedUrl !== $videoUrl and that $embedUrl begins with
the expected trusted prefix (e.g. 'https://www.youtube-nocookie.com/embed/');
only then output the iframe (using rex_escape for the src); otherwise do not
render the iframe (or render a safe fallback/message) to prevent embedding
arbitrary javascript:, data: or external sources.
In `@ytemplates/classic/value.custom_link_multi.tpl.php`:
- Line 26: $class_group is built with 'form-group', $this->getHTMLClass() and
$this->getWarningClass() but never used; change the container DIV that currently
uses $this->getHTMLClass() (the wrapper around the field in
value.custom_link_multi.tpl.php) to use $class_group instead so the Bootstrap
"form-group" and validation warning class from $this->getWarningClass() are
applied to the wrapper; ensure you keep the trimmed $class_group string as
constructed.
---
Outside diff comments:
In `@lang/fr_fr.lang`:
- Around line 68-82: Add the missing French translation for the i18n key
referenced by lib/yform/value/custom_link.php: yform_values_custom_link_anchor
is used as the label for the anchor checkbox but does not exist in
lang/fr_fr.lang; add an appropriate French string (e.g. "Bouton d'ancre" or
"Ancre") as yform_values_custom_link_anchor in the fr_fr.lang file so the
backend shows a proper label instead of the raw key.
In `@lib/MForm/Parser/MFormParser.php`:
- Around line 998-1009: The current getWidgetId(MFormItem $item) builds the
final crc32 over implode('', $varId) which causes collisions like ['1','23'] vs
['12','3']; change the concatenation to preserve segment boundaries (e.g. join
with a stable delimiter or encode the array) before computing crc32 so segments
can't merge—update getWidgetId to use a delimiter-aware join (or
json_encode($varId)) when forming the string fed to abs(crc32(...))) and keep
references to MFormItem::getVarId()/setVarId and getWidgetId in your change.
---
Minor comments:
In `@assets/css/list-widget.css`:
- Line 111: Replace the deprecated CSS property "word-break: break-word;" with
"overflow-wrap: break-word;" in the rule that currently contains "word-break:
break-word;" so the styles use the modern, standards-compliant property; update
the declaration (remove or replace the old "word-break: break-word;" line) and
keep any other existing declarations in that selector intact.
In `@assets/js/customlink.js`:
- Around line 421-425: Entferne den toten mousedown-Handler: lösche die an
multiWidget gebundene Listener-Registration multiWidget.on('mousedown.clmulti',
'.mform-cl-multi-handle', ...), da der Callback leer/auskommentiert ist und nur
unnötige Bindungen erzeugt; wenn später Drag/Sortable wirklich hinzugefügt wird,
implementiere das in einem eigenen PR und verwende dort eine dedizierte Funktion
oder Bibliothek statt dieses Platzhalers.
In `@assets/js/imglist.js`:
- Around line 138-146: The code currently hardcodes '/media/' for SVG/video in
imglist_add_img_by_last_list_item; instead read the media base URL provided by
the backend (use the data-preview-base attribute that the PHP widget should
emit, e.g. on the widget root or the media list element) and build the source as
previewBase + file (or previewBase + encodedFile) rather than '/media/<file>';
update the logic that sets url and source in imglist_add_img_by_last_list_item
to derive url = previewBase (fallback to existing index.php path if dataset
missing) and then compute source = isVideo ? (url + file) : (url + encodedFile)
so installs in subdirectories use the correct base.
In `@assets/js/list-widget.js`:
- Around line 377-390: mformListBindPopupSync currently relies on the
beforeunload event as a fallback which can miss cases where a popup is closed
via window.close(); update the fallback to start a short setInterval poll that
checks popup.closed and calls the callback (once) when true and then clears the
interval; keep the existing beforeunload handler but ensure both paths guard so
the callback only runs once (use a local called flag or clear the interval
before invoking callback), and clear the interval/handlers if popup becomes null
or an error occurs; reference the mformListBindPopupSync and
mformListInstallPopupBridge interactions to ensure compatibility with the
patched writeREXLinklist()/writeREXMedialist() bridge.
In `@assets/mform.js`:
- Around line 136-157: In applyConditional, when findSourceFields(source)
returns no fields the current logic always calls target.show(), which breaks
configurations that expect action:'hide'; change the no-fields branch in
applyConditional (the block using findSourceFields and variable action) to
handle missing source according to action: if action === 'hide' then mark/hide
the target (e.g. add 'mform-conditional-hidden' and call target.hide()),
otherwise leave it shown (or unchanged); also emit a console.warn or
console.debug mentioning the missing source and the affected target to aid
debugging (reference function applyConditional, findSourceFields, variable
action and data attribute mform-conditional-action).
In `@docs/09_templates.md`:
- Around line 13-15: Die Doku widerspricht sich zur Registry-Verantwortung:
entweder MForm liefert die Registry intern oder das Projekt muss eine eigene
Registry bereitstellen (z.B. TemplateRegistry), also korrigiere den Text so,
dass es eine einzige klare Verantwortung gibt — entweder entferne die Aussage
"MForm liefert die Registry selbst mit" und beschreibe, wie Projekte eine eigene
Registry (TemplateRegistry) registrieren und wie MForm::fromTemplate() /
->applyTemplate() diese Registry nutzen, oder belasse die Aussage und ergänze
explizit den Sonderfall, dass Projekte nur Erweiterungen/Registrierungen (key +
Klassenname) vornehmen dürfen und keine eigene Registry-Klasse erstellen dürfen;
referenziere dabei die Symbole MForm::fromTemplate, ->applyTemplate und
TemplateRegistry, damit Leser wissen, welches Verhalten erwartet wird.
In `@lib/Widget/var_custom_medialist.php`:
- Around line 79-95: The French language file is missing the i18n keys used by
the view switch (mform_list_widget_view_list and mform_list_widget_view_grid)
referenced in var_custom_medialist.php; add these two keys to fr_fr.lang with
appropriate French translations (e.g., "Liste" and "Grille" or your preferred
phrasing) so the tooltips rendered by the viewButton show translated labels
instead of raw key names; ensure the keys exactly match
mform_list_widget_view_list and mform_list_widget_view_grid and follow the same
file format/encoding as the other entries in fr_fr.lang.
In `@lib/yform/value/custom_link_multi.php`:
- Around line 50-65: In getListValue, defend against null $params['value']
before calling html_entity_decode/rex_escape: treat null the same as an empty
string and return '-' (or coerce to ''), e.g. read the incoming value with a
null-coalescing check ($value = $params['value'] ?? '') or an isset check, use
$value for html_entity_decode and rex_escape, and ensure
rex_var_custom_link::getCustomLinkText() always receives a string so no null is
passed into html_entity_decode or rex_escape.
In `@pages/module/extended/conditional_fields_builder/output.inc`:
- Line 3: Ersetze die inkonsistente Kurz-Placeholder-Nutzung und das rohe
Dumpen: tausche dump('REX_VALUE[1]') gegen das explizite Placeholder-Format
REX_VALUE[id=1] und übergebe den Platzhalterinhalt vor dem Dump an
MFormRepeaterHelper::decode() (oder die passende Decode-Methode), sodass
addConditionalFieldsetArea/Output das decodierte Struktur-Array statt des rohen
Strings erhält; referenziere hierbei die vorhandene dump-Funktion, den
Platzhalter REX_VALUE[id=1] und MFormRepeaterHelper::decode.
In `@pages/module/repeater/full_feature_lab/output.inc`:
- Around line 89-91: The check for $isDisabled is dead because
MFormRepeaterHelper::decode() (and its
prepareItemsForOutput/unset(self::DISABLED_KEY)) removes the __disabled key;
remove the dead branch: drop the $isDisabled variable and the conditional
class/label logic in pages/module/repeater/full_feature_lab/output.inc so the
panel always renders as the active state (or, if you actually need to show
disabled items, replace decode() usage with raw JSON decoding and manual
inspection of __disabled before unset); locate the code around the echo lines
that build the panel class and heading to apply the change (references:
$isDisabled, MFormRepeaterHelper::decode(), prepareItemsForOutput,
DISABLED_KEY).
In `@pages/module/repeater/tinymce_nested_repeater/output.inc`:
- Line 5: REX_VALUE[1] is being output raw inside an <h1>, which can render
untrusted HTML/JS; update the template to escape the token output (e.g., replace
REX_VALUE[1] with rex_escape(REX_VALUE[1]) or use htmlspecialchars(REX_VALUE[1],
ENT_QUOTES, 'UTF-8')) so the heading prints safe text; locate the literal
"REX_VALUE[1]" in the output template and wrap it with the chosen escaping
helper (rex_escape or htmlspecialchars) to ensure any HTML or <script> tags are
neutralized.
In `@README.de.md`:
- Around line 14-25: In the German README (Version‑9 bullet list) replace
transliterated Umlaut spellings with proper Umlaut characters (e.g.
"gefuellter"→"gefüllter", "gruen"→"grün", "unveraendert"→"unverändert" and
similar occurrences) so the user-facing docs are consistent; update the bullets
that mention addRepeaterElement(), addCustomLinkMultipleField(...),
addConditionalFieldsetArea(...), MFormRepeaterHelper::decode(), and the
value-types custom_link / custom_link_multi to use the corrected Umlaut forms.
---
Nitpick comments:
In `@assets/css/imglist.css`:
- Around line 96-114: The dark-mode custom property definitions are duplicated
in body.rex-theme-dark and the `@media` block
(body.rex-has-theme:not(.rex-theme-light)); extract these identical CSS
variables into a shared file (e.g., assets/css/_widget-tokens.css) and remove
the duplicate declarations from imglist.css so both imglist and list-widget
simply consume the shared :root/body variable definitions; update imports to
include the new _widget-tokens.css where needed.
In `@assets/js/customlink.js`:
- Around line 408-414: The rex:selectCustomLink.clmulti handler is being bound
repeatedly per Multi-Widget causing O(N^2) work; change to a single global
delegating listener bound once (instead of inside each widget init) that locates
the relevant widget via $(e.target).closest('.rex-js-cl-multi') or using the
provided input to compute ownerMulti, checks ownership (ownerMulti.length), and
then calls customLinkMultiSerialize(ownerMulti) (or the correct multiWidget
instance) for that specific widget; implement this by moving the
$(window).on('rex:selectCustomLink.clmulti', ...) registration out of the
per-widget initialization, or guard it with a module-level flag so it only
registers once, and ensure you reference the existing symbols multiWidget,
ownerMulti and customLinkMultiSerialize when wiring the call.
In `@assets/js/imglist.js`:
- Around line 130-153: The inner conditional "(listItems.length - 1) < i" inside
the loop in imglist_add_img_by_last_list_item is redundant because startIndex is
set to Math.max(0, listItems.length) and the for-loop iterates i from startIndex
to options.length - 1, so simply remove that if-check and unnest its body (the
block that creates item, computes extension, builds source/media, constructs
new_li, calls imglist_add_tooltip and appends to
element.find('ul.thumbnail-list')) so the loop body runs directly for each i;
keep all referenced symbols (startIndex, options, listItems, item, file,
extension, encodedFile, url, isVideo, source, media, new_li,
imglist_add_tooltip) intact.
In `@assets/js/list-widget.js`:
- Around line 64-73: The tiny setTimeout(…, 2) is a fragile race workaround in
the rex:ready handler around mform_list_widget; remove the magic delay and
either (a) document why a delay was needed as an inline comment or (b) make the
init defensive: in the rex:ready handler or inside mformListWidgetInit check for
the real prerequisites (e.g. existence of global helpers like writeREXMedialist
and writeREXLinklist or that necessary DOM nodes/scripts are available) and
skip/retry initialization only when those prerequisites are present; update
container.find(mform_list_widget).each(...) to use that defensive check instead
of relying on timeout so the race is handled explicitly.
- Around line 293-309: mformListRender currently mutates option:selected (sets
first option selected) which causes side effects; remove that mutation from
mformListRender and instead perform it once during initialization (e.g. a new
mformListInit or guarded branch) so render stays idempotent; implement the init
by checking/setting a widget-scoped flag (data- or class-based) and move the
logic that currently sets select.find('option').first().prop('selected', true)
into that init path, keeping mformListWriteHidden untouched and continuing to
build hidden value from all options.
- Around line 226-231: The current regex alternatives in the baseId extraction
use /^REX_(?:MEDIA|MEDIALIST|LINKLIST)_/ and
/^REX_(?:MEDIA|MEDIALIST|LINKLIST)_SELECT_/ which rely on backtracking and can
silently produce wrong baseId for inputs like "REX_MEDIALIST_…"; update both
patterns to list the longest alternative first (e.g. MEDIALIST before MEDIA) so
they become /^REX_(?:MEDIALIST|MEDIA|LINKLIST)_/ and
/^REX_(?:MEDIALIST|MEDIA|LINKLIST)_SELECT_/; adjust the occurrences in the code
paths that inspect hiddenId and selectId (variables baseId, hiddenId, selectId)
so the replacement logic remains identical but more robust.
- Around line 100-159: The event binding on widget.find('.mform-list-btn') will
attach handlers to buttons inside nested list-widgets; change the selector to
limit to toolbar buttons of the current widget (e.g. use a direct-child selector
like '> .mform-list-toolbar .mform-list-btn' instead of
widget.find('.mform-list-btn')) or add a guard in the handler that checks
$(this).closest('.mform-list-widget').is(widget) before proceeding; update the
binding line that calls
widget.find('.mform-list-btn').off('click.mformListWidget').on('click.mformListWidget',
...) and/or add the closest-check at the start of that handler so only buttons
belonging to the current mform-list-widget trigger
mformListRender/mformListWriteHidden and the various REX* calls.
- Around line 273-287: The code in mformListBuildOptionsFromHidden currently
hardcodes the German prefix "Artikel " for linklist fallbacks; change it to read
a configurable/localizable prefix instead: inside the loop where type ===
'linklist' replace the fixed string with a lookup from the widget's
data-attribute (e.g. widget.attr('data-label-prefix')) and fallback to a
localized string via rex_i18n (or an empty string) if the attribute is missing,
so the code uses that prefix + value (or just value when prefix is empty);
ensure you update the references around type === 'linklist' and any surrounding
logic that builds the option text to use this new prefix variable.
In `@assets/mform.js`:
- Around line 96-134: The function name and parameter both use compareValue
which causes a confusing name collision; rename either the function compareValue
(e.g., to evaluateCondition) or the parameter compareValue (e.g., to expected or
compareTo) and update all references accordingly: change the declaration
function compareValue(sourceValue, compareValue, operator) to the new names and
replace every use of the parameter inside the function (all occurrences of
compareValue -> expected/compareTo) and update any external callers that invoke
compareValue(...) to the new function name if you renamed the function; preserve
existing logic and tests while running a global search/replace to ensure no
stale identifiers remain.
- Around line 165-167: Aktuelles Binding
mform.off(...).on('change.mformConditional input.mformConditional', ':input',
...) feuert evaluateAllConditionals() doppelt für Texteingaben; trenne die
Handler: binde 'change.mformConditional' an inputs vom Typ select/checkbox/radio
(z.B. selector 'select, input[type=checkbox], input[type=radio]') und binde
'input.mformConditional' nur an textartige Felder (z.B. 'input[type=text],
input[type=search], textarea'), oder alternativ wrappe den Aufruf von
evaluateAllConditionals() in einen kleinen Debounce in der event-Callback; passe
die bestehende mform.off(...).on(...)-Aufrufe entsprechend an und rufe weiter
evaluateAllConditionals() aus diesen separaten Callbacks auf.
In `@docs/03_customlink.md`:
- Around line 38-65: Add a short cross-reference in the "Multiple-Variante
(Repeater-basiert)" section to instruct consumers to decode stored JSON via the
central helper: mention MFormRepeaterHelper::decode() (instead of json_decode)
and show that addCustomLinkMultipleField(...) stores a JSON array which should
be passed through MFormRepeaterHelper::decode() in module output (mirror the
Output notes used for other repeater fields so users use the helper
consistently).
- Around line 162-178: The example output loop assumes every $link is a
non-empty string; update the foreach over $links to skip empty items by adding a
guard that casts $link to string, trims it, and continues when it equals ''.
Place this check before calling MFormOutputHelper::getCustomUrl and
MFormOutputHelper::prepareCustomLink so empty entries do not produce empty <a
href=""> output.
In `@docs/08_mblock_migration.md`:
- Around line 131-134: The constants MODULE_ID, SOURCE_COLUMN, TARGET_COLUMN and
DRY_RUN are declared with const which will throw "Cannot redeclare constant" if
the snippet is included more than once; change those declarations to conditional
definitions using PHP's defined(...) ? : define(...) pattern (e.g. if
(!defined('MODULE_ID')) define('MODULE_ID', 123)) to make the snippet safe for
multiple includes, and apply the same fix to the second script block that
declares the same constants (the block around the later constants such as
MODULE_ID, SOURCE_COLUMN, TARGET_COLUMN, DRY_RUN).
- Around line 175-188: The silent suppression operator on the unserialize call
should be replaced with explicit error handling: remove the "@" from
unserialize($raw, ['allowed_classes' => false]) and wrap the call in a short
set_error_handler / restore_error_handler block that converts warnings/notices
into an ErrorException (or use try/catch for PHP 8+ where appropriate), then
restore the previous handler and handle failures by checking the return and
falling back to returning []; update both occurrences (the current unserialize
usage and the second copy around line 366) and add a brief comment stating why
we deliberately convert warnings rather than silencing them.
In `@fragments/mform/mform_select.php`:
- Around line 5-7: Der Fall-Through von case 'multiselect' in case 'select' ist
beabsichtigt; füge direkt nach der Zeile mit $this->setVar('attributes',
$this->getVar('attributes') . ' multiple', false); einen klaren Kommentar wie
"/* fall through */" oder "// intentinal fall-through" hinzu, damit statische
Analyse-Tools und Leser erkennen, dass das Verhalten gewollt ist (beziehe dich
auf die switch cases 'multiselect' und 'select' und die setVar('attributes',
...) Aufruf).
In `@lib/MForm/Repeater/MFormRepeaterHelper.php`:
- Around line 86-102: The switch branch for case 'repeater' intentionally falls
through into case 'close-repeater', but this is not documented; add an explicit
fall-through comment (e.g. "// fall through to close-repeater: add group/parent
attributes") directly after the repeater block or use a clear "// no break"
marker so future maintainers know the fallthrough is deliberate; update the
block around the 'repeater' case that manipulates $mformItem and calls
self::prepareChildItems(...) to include this comment so that the subsequent
'close-repeater' case that adds group/groups/parent_id is obviously intentional.
- Around line 246-263: In decode(), detect unexpanded REDAXO placeholders in
$rexValue (e.g. pattern /REX_(VALUE|INPUT)\[.*?\]/) before json_decode; if
rex::isDebugMode() is true, emit a clear warning (via rex_logger::warning or
trigger_error) including the raw $rexValue and context (function decode) so
developers see accidental non-substitution, but still return the empty array as
now; keep existing flow and call sites intact (functions/methods to edit:
MFormRepeaterHelper::decode, variables $rexValue, $normalizedValue, and final
return to prepareItemsForOutput).
- Around line 265-297: Die aktuelle isRepeaterItemList() erkennt Arrays
fälschlich als Listen, wenn nur die Werte Arrays sind; ändere die Funktion so
sie zusätzlich überprüft, dass die Schlüssel numerisch-sequentiell sind (z.B.
array_keys($value) === range(0, count($value)-1)) und nur dann true zurückgibt
(leere Arrays können weiterhin true bleiben); so verhindert
prepareItemsForOutput() (und das Entfernen von self::DISABLED_KEY) das
Rekursions-Verarbeiten assoziativer Felder wie in prepareItemsForOutput().
In `@lib/MForm/Utils/LayoutPreviewBuilder.php`:
- Line 110: Entferne den veralteten TODO-Kommentar über der bereits
implementierten Methode addArrow in der Klasse LayoutPreviewBuilder (Datei
enthält die Methode addArrow); der Kommentar "hier sollte eine Methode
addArrow() hinzugefügt werden, die die Pfeile hinzufügt" ist obsolet und sollte
gelöscht, nicht modifiziert, werden, damit der Code sauber bleibt.
- Around line 266-280: The render() method assumes aspectRatio is "W:H" and
blindly explodes and divides by $w causing notices/div-by-zero; update render()
to reuse the same validation as getHeight() (parse and validate
$this->aspectRatio contains ':' , both sides are numeric/positive and $w != 0)
and fall back to a safe default or to getHeight() when validation fails; adjust
the calculation that sets $maxHeight = ($this->svgWidth / $w) * $h to only run
after successful validation (or compute $maxHeight via $this->getHeight() if you
prefer) so render() no longer risks division-by-zero or notices.
In `@lib/MForm/Utils/MFormItemManipulator.php`:
- Around line 25-32: The code allows an int 0 to reach htmlspecialchars() and
should convert values to string first; update the conditional to also check for
numeric zero (add $value !== 0) and when calling $item->setValue(...) cast
inputs to string: use (string)$value in the first setValue call and
(string)$item->getStringValue() in the second, keeping the existing checks and
htmlspecialchars(...) wrapping the casted string.
In `@lib/MFormTemplate/TemplateRegistry.php`:
- Around line 46-56: The apply() method silently returns the original MForm when
the given $key is not registered; change it to throw a rex_exception instead so
callers get immediate feedback on invalid template keys: in
TemplateRegistry::apply(MForm $form, string $key, array $context = []) check
existence with self::hasTemplate($key) (or test self::$templates[$key]) and
throw a rex_exception with a clear message including the invalid $key if not
found (consistent with register()); then instantiate $templateClass and call
$template->apply($form, $context) as before.
In `@lib/Widget/var_custom_link_multi.php`:
- Around line 43-47: Die foreach-Schleife verwendet die nicht genutzte
Index-Variable $i; ersetze die Signatur foreach ($links as $i => $linkValue)
durch eine reine Werteschleife foreach ($links as $linkValue) in der
betreffenden Stelle (siehe Verwendung von $links, $linkValue,
rex_var_custom_link::getWidget und self::wrapItem) und prüfe, dass $i sonst
nirgends mehr referenziert wird, damit PHPMD-Warnungen wegfallen.
- Around line 37-39: Die Verwendung des schwachen Platzhalters 'CMLIDX' bei
rex_var_custom_link::getWidget(...) führt zu möglichen Kollisionen beim
JavaScript-Replace; ändere die Template-Generierung in var_custom_link_multi.php
so dass ein eindeutigere Token (z.B. '__MFORM_CL_MULTI_IDX__') oder besser noch
ein data-template-id Attribut verwendet wird statt globaler String-Replacement.
Passe die Stelle an, die $templateHtml =
rex_var_custom_link::getWidget('CMLIDX', ...) erstellt, und aktualisiere die
JS-Logik (die derzeit template.split('CMLIDX').join(idx) verwendet), sodass nur
das sichere Token oder das data-template-id Feld ersetzt/ausgelesen wird (oder
das neue id-Attribut per DOM-Klonen gesetzt wird), um ungewollte Ersetzungen in
$args (z.B. btn_add) zu vermeiden.
In `@package.yml`:
- Line 35: The dependency constraint mblock: '>=0.0.0' in package.yml currently
prevents any mblock versions from being selected; update the constraint to the
intended range (for example a permissive upper-bound like '<999.0.0' or a
concrete semantic range such as '>=1.2.0 <2.0.0') or, if exclusion is
intentional, replace the line with a clear comment explaining that mblock must
not coexist with mform; locate the mblock entry in package.yml and change the
version string or add the explanatory comment accordingly.
In `@pages/docs.php`:
- Around line 89-96: The current file hacks around HTML-encoded query keys by
checking for 'amp;func', stripping 'amp;' with preg_replace and falling back to
'basics' — instead locate and fix the code that emits those backend links so
query params are not HTML-encoded (use rex_url::currentBackendPage([...]) when
building links) and remove the brittle fallback logic: eliminate the
special-case rex_request('amp;func', ...) and the preg_replace('/^amp;/', '',
$func) handling and ensure callers pass a normal 'func' param that is validated
against $mform_doc_pages.
- Around line 270-300: The copy-button currently hardcodes German labels and
lacks clipboard error handling; update the HTML generation to inject i18n
strings (e.g., rex_i18n::msg("copy"), rex_i18n::msg("copied"),
rex_i18n::msg("copy_error")) into data attributes on the code block containers,
then change the JS in the document.querySelectorAll(".rex-docs pre") loop and
the btn click handler to read btn.dataset.* for the labels instead of hardcoded
"Kopieren"/"✓ Kopiert" and add a .catch on navigator.clipboard.writeText(...) to
set an error state (change text to the localized error label, add an error class
and/or console.error) so users get feedback when copying fails.
- Around line 100-136: The search loop reads every Markdown with rex_file::get
on each request (inside the loop over $mform_doc_pages) which is expensive;
replace on-the-fly reads with cached content retrieval (e.g. rex_file::getCache
or a rex_cache keyed by the file path plus filemtime or the addon language) and
use that cached content in place of $contentRaw, or build and consult a simple
persistent index per backend language keyed by $q to avoid scanning all files;
update the code that sets $contentRaw (the rex_file::get call) to fetch from
cache first and fall back to reading and storing the cache, keeping the rest of
the snippet/heading/anchor logic unchanged (references: $mform_doc_pages,
$contentRaw, rex_file::get, $searchResults).
In `@pages/index.php`:
- Around line 8-12: The current check uses in_array($part2, ['info'], true) to
exclude the "info" page; replace this with a direct strict comparison against
the string returned by rex_be_controller::getCurrentPagePart(2) (i.e., use
$part2 !== 'info') so the intention is clearer and avoids creating a one-element
array — update the conditional that surrounds echo rex_view::title(...)
accordingly while preserving strict type comparison.
In `@pages/module/extended/conditional_fields/output.inc`:
- Around line 39-41: Die Ausgabe von $text (REX_VALUE[4]) erfolgt aktuell
ungefiltert in echo '<div class="text-content">' . $text . '</div>'; — füge
unmittelbar vor dieser Ausgabe einen erklärenden Kommentar hinzu, der
klarstellt, dass dieses Feld Rich-Text/TinyMCE-HTML enthält und deshalb bewusst
nicht mit rex_escape() escaped wird, und vermerke außerdem, dass für
Plaintext-Felder rex_escape() verwendet werden muss; wenn dieses Feld in Zukunft
aus unsicheren Quellen befüllbar ist, weise auf die Notwendigkeit zusätzlicher
Validierung/Sanitization hin (referenzen: variable $text / RE X_VALUE[4] im
aktuellen Block).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c496935-7304-4c0c-9b50-a04d412b086b
⛔ Files ignored due to path filters (1)
assets/js/sortable.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (75)
CHANGELOG.mdREADME.de.mdREADME.mdassets/css/flex-repeater.cssassets/css/imglist.cssassets/css/list-widget.cssassets/css/mform.cssassets/js/customlink.jsassets/js/flex-repeater.jsassets/js/imglist.jsassets/js/list-widget.jsassets/mform.jsboot.phpdocs/01_basics.mddocs/02_redaxo.mddocs/03_customlink.mddocs/05_wrapper.mddocs/06_advanced.mddocs/07_repeater.mddocs/08_mblock_migration.mddocs/09_templates.mdfragments/mform/mform_base.phpfragments/mform/mform_select.phpfragments/mform/mform_wrapper.phplang/de_de.langlang/en_gb.langlang/fr_fr.langlib/MForm.phplib/MForm/FlexRepeater/MFormFlexRepeaterRenderer.phplib/MForm/MFormElements.phplib/MForm/Parser/MFormParser.phplib/MForm/Repeater/MFormRepeaterHelper.phplib/MForm/Utils/HtmlToSvgConverter.phplib/MForm/Utils/LayoutPreviewBuilder.phplib/MForm/Utils/MFormItemManipulator.phplib/MFormTemplate/TemplateInterface.phplib/MFormTemplate/TemplateRegistry.phplib/Widget/var_custom_link.phplib/Widget/var_custom_link_multi.phplib/Widget/var_custom_linklist.phplib/Widget/var_custom_medialist.phplib/yform/value/custom_link.phplib/yform/value/custom_link_multi.phppackage.ymlpages/demo.demo_community.phppages/docs.phppages/index.phppages/module/base/select/output.incpages/module/base/text/output.incpages/module/expert/html_form_elements/output.incpages/module/extended/conditional_fields/input.incpages/module/extended/conditional_fields/output.incpages/module/extended/conditional_fields_builder/input.incpages/module/extended/conditional_fields_builder/output.incpages/module/extended/placeholder/output.incpages/module/repeater/full_feature_lab/input.incpages/module/repeater/full_feature_lab/output.incpages/module/repeater/nested_repeater/input.incpages/module/repeater/nested_repeater/output.incpages/module/repeater/single_repeater/input.incpages/module/repeater/single_repeater/output.incpages/module/repeater/tinymce_nested_repeater/input.incpages/module/repeater/tinymce_nested_repeater/output.incpages/module/repeater/widgets_repeater/output.incpages/module/wrapper/accordion/output.incpages/module/wrapper/collapse/output.incpages/module/wrapper/collapse_checkradio/output.incpages/module/wrapper/collapse_select/output.incpages/module/wrapper/columns/output.incpages/module/wrapper/inline/output.incpages/module/wrapper/tabs/output.incytemplates/bootstrap/value.custom_link.tpl.phpytemplates/bootstrap/value.custom_link_multi.tpl.phpytemplates/classic/value.custom_link.tpl.phpytemplates/classic/value.custom_link_multi.tpl.php
💤 Files with no reviewable changes (1)
- pages/demo.demo_community.php
There was a problem hiding this comment.
Pull request overview
This PR upgrades mform to v9.0.0 and introduces a new Flex Repeater runtime as the default, adds new widgets (linklist/medialist + multi custom-link), and expands the template + conditional-field APIs alongside refreshed demos and documentation.
Changes:
- Introduces Flex Repeater UI/runtime (incl. copy/paste, per-item active/offline state, improved TinyMCE handling) and
MFormRepeaterHelper::decode()for output filtering. - Adds new widgets and value types:
custom_link_multi, repeater-based multi custom links, and new list widgets for medialist/linklist (view switch + previews). - Adds Conditional Fieldset API and Template Registry API; revamps backend docs navigation/search/TOC and updates docs/demos/readmes.
Reviewed changes
Copilot reviewed 75 out of 76 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| ytemplates/classic/value.custom_link.tpl.php | Fixes classic custom_link params; adds anchor option. |
| ytemplates/classic/value.custom_link_multi.tpl.php | Adds classic YForm template for multi custom links. |
| ytemplates/bootstrap/value.custom_link.tpl.php | Adds anchor option to bootstrap custom_link template. |
| ytemplates/bootstrap/value.custom_link_multi.tpl.php | Adds bootstrap YForm template for multi custom links. |
| README.md | Updates v9 release notes + migration example link. |
| README.de.md | German v9 release notes; fixes typos and installation text. |
| pages/module/wrapper/tabs/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/wrapper/inline/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/wrapper/columns/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/wrapper/collapse/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/wrapper/collapse_select/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/wrapper/collapse_checkradio/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/wrapper/accordion/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/repeater/widgets_repeater/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/repeater/tinymce_nested_repeater/output.inc | New TinyMCE nested repeater output demo. |
| pages/module/repeater/tinymce_nested_repeater/input.inc | New TinyMCE nested repeater input demo. |
| pages/module/repeater/single_repeater/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/repeater/single_repeater/input.inc | Updates repeater options (collapsed/toggle-all). |
| pages/module/repeater/nested_repeater/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/repeater/nested_repeater/input.inc | Updates nested repeater options (collapsed/toggle-all). |
| pages/module/repeater/full_feature_lab/output.inc | New “full feature lab” output demo (debug view). |
| pages/module/repeater/full_feature_lab/input.inc | New “full feature lab” input demo (tabs + widgets + copy/paste). |
| pages/module/extended/placeholder/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/extended/conditional_fields/output.inc | New conditional-fields output demo. |
| pages/module/extended/conditional_fields/input.inc | New conditional-fields input demo. |
| pages/module/extended/conditional_fields_builder/output.inc | New conditional-fields builder output stub. |
| pages/module/extended/conditional_fields_builder/input.inc | New conditional-fields builder input demo. |
| pages/module/expert/html_form_elements/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/base/text/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/module/base/select/output.inc | Demo output uses MFormRepeaterHelper::decode(). |
| pages/index.php | Simplifies title banner handling for backend pages. |
| pages/docs.php | Rebuilds docs page (sidebar nav/search/TOC + markdown rendering). |
| pages/demo.demo_community.php | Removes duplicated title rendering (now centralized). |
| package.yml | Bumps version to 9.0.0; updates subpages; adds mblock conflict. |
| lib/yform/value/custom_link.php | Adds anchor checkbox definition to YForm value type. |
| lib/yform/value/custom_link_multi.php | Adds new YForm value type custom_link_multi. |
| lib/Widget/var_custom_medialist.php | Adds new medialist widget (previews + view switch). |
| lib/Widget/var_custom_linklist.php | Adds new linklist widget (repeater-compatible list UI). |
| lib/Widget/var_custom_link.php | Adjusts anchor enabling default behavior. |
| lib/Widget/var_custom_link_multi.php | Adds multi custom-link widget (JSON array storage). |
| lib/MFormTemplate/TemplateRegistry.php | Adds internal template registry for defaults/templates. |
| lib/MFormTemplate/TemplateInterface.php | Adds template interface for registry templates. |
| lib/MForm/Utils/MFormItemManipulator.php | Refactors value/default handling and guards double processing. |
| lib/MForm/Utils/LayoutPreviewBuilder.php | Adds/updates docblocks for SVG preview builder. |
| lib/MForm/Utils/HtmlToSvgConverter.php | Adds/updates docblocks for SVG converter. |
| lib/MForm/Repeater/MFormRepeaterHelper.php | Adds decode/filter helpers and __disabled handling. |
| lib/MForm/MFormElements.php | Adds conditional fieldsets, flex repeater, multi custom-link API. |
| lib/MForm/FlexRepeater/MFormFlexRepeaterRenderer.php | Adds template renderer for flex repeater (HTML template generation). |
| lib/MForm.php | Adds template registry API (registerTemplate, fromTemplate, applyTemplate). |
| lang/fr_fr.lang | Adds missing docs label translation key. |
| lang/en_gb.lang | Adds translations for new demos, repeater UI, docs UI, yform value types. |
| lang/de_de.lang | Adds German translations for new demos, repeater UI, docs UI, yform value types. |
| fragments/mform/mform_wrapper.php | Fixes switch-case syntax (; → :). |
| fragments/mform/mform_select.php | Fixes switch-case syntax (; → :). |
| fragments/mform/mform_base.php | Adjusts description layout columns. |
| docs/09_templates.md | New docs: template/defaults registry usage. |
| docs/08_mblock_migration.md | New migration guide from MBlock to MForm 9. |
| docs/07_repeater.md | Expands repeater docs (decode, tinyMCE nested, copy/paste, options). |
| docs/06_advanced.md | Updates advanced docs (conditional fields, new APIs, previews, templates). |
| docs/05_wrapper.md | Documents showWrapper / setShowWrapper. |
| docs/03_customlink.md | Adds multi custom-link + yform value type docs + anchor info. |
| docs/02_redaxo.md | Reworks docs for media/link widgets and new list widgets. |
| docs/01_basics.md | Fixes wording; adds notice/help-block example. |
| CHANGELOG.md | Adds 9.0.0 changelog section. |
| boot.php | Switches to flex-repeater assets; adds list-widget + CSS. |
| assets/mform.js | Adds conditional-fieldset JS initialization. |
| assets/js/list-widget.js | New list-widget behavior (popup sync, render, view toggle). |
| assets/js/imglist.js | Refactors imglist init + sortable guard + accessibility/empty-state handling. |
| assets/js/customlink.js | Improves customlink behavior; adds custom-link-multi JS. |
| assets/css/mform.css | Updates custom-link styling + adds custom-link-multi styles. |
| assets/css/list-widget.css | Adds list-widget styling including grid/list view. |
| assets/css/imglist.css | Redesigns imagelist styling + adds empty-state visuals. |
| assets/css/flex-repeater.css | Adds full flex-repeater styling (UI, nested, copy/paste, dark mode). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/MForm/MFormElements.php (2)
333-402:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
addRadioImgField/addRadioIconField/addRadioColorField:?array $options = null+ ungeschütztesforeach.Die Parameter wurden auf
?array $options = nullaufgeweicht, aber alle drei Methoden iterieren direkt mitforeach ($options as ...). Wird eine dieser Methoden ohne$options(oder mitnull) aufgerufen, wirft PHP 8 einenTypeError. Entweder den Default auf[]setzen oder defensiv prüfen.🛡️ Vorschlag
- public function addRadioImgField(float|int|string $id, ?array $options = null, ?array $attributes = null, ?string $defaultValue = null): MForm - { - $newOptions = []; - - foreach ($options as $key => $option) { + public function addRadioImgField(float|int|string $id, ?array $options = null, ?array $attributes = null, ?string $defaultValue = null): MForm + { + $newOptions = []; + + foreach ((array) $options as $key => $option) {Analog für
addRadioIconField(Zeile 371) undaddRadioColorField(Zeile 387).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/MForm/MFormElements.php` around lines 333 - 402, The three methods addRadioImgField, addRadioIconField, and addRadioColorField declare ?array $options = null but immediately foreach over $options, causing a TypeError when null is passed; fix by making $options default to an empty array (change signature to array $options = []) or add a defensive early guard (e.g., if ($options === null) $options = []; then proceed) and keep existing logic; update all three methods (addRadioImgField, addRadioIconField, addRadioColorField) to use the same fix so foreach never iterates over null.
168-175:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMoin –
addTabElementruftaddForm()mit falschen Argumenten auf.Die Signatur von
addForm()istaddForm($form, bool $parse, bool $debug, bool $showWrapper). Im Aufruf hier (->addForm($form, $parse, $showWrapper)) wird$showWrapperals drittes Argument geliefert und landet damit in$debug;$showWrapperselbst bleibt auf seinem Defaultfalse. Damit aktiviert ein Aufrufer mitshowWrapper: trueversehentlich den Debug-Modus, und der Wrapper-Render wird nie eingeschaltet.Alle anderen Wrapper-Methoden (
addFieldsetAreaZeile 146,addColumnElement156,addInlineElement164,addCollapseElement191,addConditionalFieldsetArea218) machen es konsistent:->addForm($form, $parse, false, $showWrapper).🐛 Vorschlag
public function addTabElement(string $label = '', mixed $form = null, bool $openTab = false, bool $pullNaviItemRight = false, array $attributes = [], bool $parse = false, bool $showWrapper = false): MForm { $attributes = array_merge($attributes, ['data-group-open-tab' => $openTab, 'pull-right' => $pullNaviItemRight]); return $this->addElement('tab', null, null, $attributes) ->setLabel($label) - ->addForm($form, $parse, $showWrapper) + ->addForm($form, $parse, false, $showWrapper) ->addElement('close-tab', null, null, $attributes); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/MForm/MFormElements.php` around lines 168 - 175, The addTabElement method calls addForm($form, $parse, $showWrapper) which passes $showWrapper into the $debug parameter of addForm; update the call in addTabElement so the third argument is false and the fourth is $showWrapper (i.e. ->addForm($form, $parse, false, $showWrapper)) to match the addForm signature and keep behavior consistent with addFieldsetArea, addColumnElement, addInlineElement, addCollapseElement, and addConditionalFieldsetArea.
🧹 Nitpick comments (7)
pages/module/extended/conditional_fields/output.inc (1)
47-47: 💤 Low valuePfad-Bestandteile in
srczusätzlich escapen.Moin –
rex_url::media($image)liefert eine konstruierte URL ohne HTML-Escape. Da$imageausREX_MEDIA[id=5]kommt und der Dateiname theoretisch Sonderzeichen enthalten könnte, istrex_escape()um die URL die robustere Variante (Defense-in-Depth, gleiche Konvention wie Zeile 63 für das Iframe).🛡️ Vorschlag
- echo '<img src="' . rex_url::media($image) . '" alt="' . rex_escape($altText) . '">'; + echo '<img src="' . rex_escape(rex_url::media($image)) . '" alt="' . rex_escape($altText) . '">';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/module/extended/conditional_fields/output.inc` at line 47, Der img src verwendet rex_url::media($image) ohne HTML-Escaping; wrappe die URL-Ausgabe mit rex_escape() analog zur Iframe-Zeile (also rex_escape(rex_url::media($image))) und behandle weiterhin $altText wie bisher; aktualisiere die echo in pages/module/extended/conditional_fields/output.inc so dass rex_url::media($image) vor dem Einfügen in src mittels rex_escape gesichert wird, um Pfad-/Dateinamen-Sonderzeichen robust zu escapen.assets/js/customlink.js (1)
427-431: 💤 Low valueToter
mousedown-Handler.Moin – Der Handler enthält nur Kommentare und tut effektiv nichts. Entweder die geplante Sortable-Integration jetzt einbauen (z. B. analog
assets/js/list-widget.jsmitwindow.Sortable) oder den leeren Handler entfernen, damit er nicht später als toter Code Verwirrung stiftet.♻️ Minimaler Cleanup
- // Sortable via move-up/move-down on drag handle click (simple swap) - multiWidget.on('mousedown.clmulti', '.mform-cl-multi-handle', function (e) { - // Only drag-to-reorder is complex; skip for now – dragging not wired without dragula/sortable - // A future improvement can add sortable library here - }); + // TODO: Sortable-Integration für .mform-cl-multi-handle (analog list-widget.js).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/js/customlink.js` around lines 427 - 431, The mousedown handler registered on multiWidget ('mousedown.clmulti' for selector '.mform-cl-multi-handle') is dead code—either remove the empty multiWidget.on('mousedown.clmulti', '.mform-cl-multi-handle', ...) handler entirely, or replace it with a real sortable integration: initialize window.Sortable on the same multiWidget container (mirroring the approach in assets/js/list-widget.js), wire the drag handle selector '.mform-cl-multi-handle', and implement the move/swap logic in the Sortable callbacks; prefer removal for minimal cleanup, or implement Sortable if you want drag-to-reorder now.assets/js/list-widget.js (2)
64-73: 💤 Low value
setTimeout(fn, 2)ohne Kommentar.Moin – Das deferred Init mit 2 ms wirkt wie eine Race-Condition-Notbremse (z. B. damit andere
rex:ready-Handler erst Markup mounten können). Bitte einen kurzen Kommentar ergänzen, warum gerade 2 ms /setTimeoutnötig ist – sonst wird das beim nächsten Refactor ohne Verständnis entfernt.📝 Vorschlag
$(document).on('rex:ready', function (e, container) { - setTimeout(function () { + // Defer ein Tick, damit andere rex:ready-Handler (z. B. Repeater) das Markup + // erst mounten können, bevor wir die Widgets initialisieren. + setTimeout(function () { if (!container || !container.find(mform_list_widget).length) { return; } container.find(mform_list_widget).each(function () { mformListWidgetInit($(this)); }); }, 2); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/js/list-widget.js` around lines 64 - 73, The short timeout (setTimeout(..., 2)) in the rex:ready handler is being used as a deliberate micro-defer to let other rex:ready handlers or async markup mounting complete before initializing list widgets; add a clear comment immediately above this setTimeout explaining that intent (mentioning the race with other rex:ready handlers/markup mounting and why a minimal delay of 2ms was chosen) and reference the related symbols mform_list_widget and mformListWidgetInit so future maintainers do not remove it without validating ordering; optionally note that a safer alternative (e.g., explicit event or requestAnimationFrame) could be considered if refactoring.
456-469: ⚡ Quick winPolling
popup.closedist der zuverlässigere Ansatz für Popup-Schließung.Moin – deine Analyse ist korrekt:
beforeunloadist für die Erkennung von Popup-Schließungen unzuverlässig, besonders auf Mobile und bei Cross-Origin-Popups. Das aktuelletry/catchmitsetTimeout(callback, 80)ist nur ein passiver Fallback und garantiert nicht, dass der Callback ausgelöst wird, wenn das Popup geschlossen wird.Das Polling auf
popup.closedist dagegen die empfohlene Methode – es funktioniert sowohl Same-Origin als auch Cross-Origin (über dieWindowProxy-Referenz), und mittry/catchbleibt es robust. Dein Vorschlag mitsetIntervalundpopup.closedist ein sauberes Upgrade, das echte Schließungserkennung bietet statt blinder Wartezeiten.♻️ Vorschlag
function mformListBindPopupSync(popup, callback) { if (!popup || typeof callback !== 'function') { callback(); return; } try { popup.addEventListener('beforeunload', function () { window.setTimeout(callback, 40); }); + // Fallback: popup.closed polling für zuverlässigere Erkennung + const watchdog = window.setInterval(function () { + try { + if (popup.closed) { + window.clearInterval(watchdog); + callback(); + } + } catch (err) { + window.clearInterval(watchdog); + } + }, 250); } catch (e) { window.setTimeout(callback, 80); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/js/list-widget.js` around lines 456 - 469, Replace the unreliable beforeunload handler in mformListBindPopupSync with a polling loop that checks popup.closed: if popup is falsy or callback is not a function keep early return; otherwise start a setInterval (e.g. 100ms) that in a try/catch reads popup.closed and when true clears the interval and invokes callback; in the catch branch (access errors) treat as still-open until closed or clear interval and call callback if popup reference becomes unusable, and ensure you clear the interval on callback to avoid leaks.assets/css/mform.css (1)
737-747: 💤 Low value
!important-Häufung bei.mform-cl-multi-remove.Die
!important-Marker aufborder,background,colorundpaddingsind fragil – sie überschreiben pauschal alles und erschweren später feines Theming. Wenn das nur dazu dient, generische.btn-Defaults zu überschreiben, würde eine spezifischere Selector-Kette (z. B..mform-cl-multi-item .mform-cl-multi-remove.btn) ohne!importantreichen.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/css/mform.css` around lines 737 - 747, Die CSS-Regeln für .mform-cl-multi-remove verwenden mehrere !important-Marker; stattdessen entferne die !important-Spezifizierer von border, background, color und padding und steigere die Selektorspezifität (z. B. .mform-cl-multi-item .mform-cl-multi-remove.btn oder ähnliche Kette) so dass die Regeln gezielt generische .btn-Defaults überschreiben; passe dabei auch die Hover-Regel (.mform-cl-multi-remove:hover) analog an (ohne !important) und teste, dass Styling weiterhin greift und kein unerwünschtes Vererben auf andere Buttons passiert.assets/js/flex-repeater.js (1)
319-323: 💤 Low valueHinweis:
:scope .mfr-nested-repeaterist breiter als der frühere Selector.Die Schwesterfunktion
collectNestedItemsarbeitet sauber mit:scope > .mfr-nested-item. Hier incollectItemDataist der Selector:scope .mfr-nested-repeater(Descendant) gefolgt von einemclosest('.mfr-item') !== itemEl-Filter. Bei der aktuellen Zwei-Ebenen-Architektur (Level 1 + Level 2) funktioniert das, aber die Logik ist subtil: Ein hypothetisches Level-3-Nested-Repeater innerhalb eines Level-2-Items hätte als nächstes.mfr-itemimmer noch das Level-1-Item und würde hier fälschlich eingesammelt werden, da.mfr-nested-itemnicht zu.mfr-itemmatcht. Das ist heute kein Bug, weil Level 3 nicht entsteht – aber falls die Architektur jemals erweitert wird, lohnt sich hier ein engerer Selector wie:scope > .mfr-item-body .mfr-nested-repeateroder ein expliziter „nicht in einem .mfr-nested-item"-Filter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/js/flex-repeater.js` around lines 319 - 323, Die aktuelle collectItemData-Logik sucht mit ':scope .mfr-nested-repeater' und filtert per nested.closest('.mfr-item') !== itemEl, was bei zusätzlicher Nesting-Tiefe falsch-positive Treffer erlauben kann; passe den Selector in collectItemData so an, dass nur direkte Level‑2-Repeater erwischt werden (z.B. ':scope > .mfr-item-body .mfr-nested-repeater' oder kombiniere ':scope .mfr-nested-repeater:not(.mfr-nested-item *)') und entferne/vereinfache das closest-Filter-Workaround; referenziere collectItemData, collectNestedItems und die querySelectorAll-Callsite beim Patch.lib/Widget/var_custom_linklist.php (1)
47-47: 💤 Low valueMoin:
rex::requireUser()vs.rex::getUser()– Inkonsistenz bei WidgetsDas sibling-Widget
var_imglist.php:93nutzt das robustere Pattern mitrex::getUser()->getComplexPerm('media')->hasMediaPerm(), währendvar_custom_linklist.php:47undvar_custom_medialist.php:55mitrex::requireUser()arbeiten. Letzterer wirft eine Exception, falls kein Backend-User aktiv ist.
rex::getUser()mit null-check ist defensiver und würde die Buttons konsistent verhindern statt zu crashen – besonders relevant, wenn die Widgets jemals in unerwarteten Kontexten gerendert werden. Für Konsistenz mitvar_imglist.phpkönnte hier angepasst werden.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/Widget/var_custom_linklist.php` at line 47, Replace the use of rex::requireUser() with a defensive null-checked rex::getUser() call so the widget does not throw when no backend user exists; specifically, in var_custom_linklist.php update the permission check that currently calls rex::requireUser()->getComplexPerm('structure')->hasStructurePerm() to instead obtain $user = rex::getUser(); if $user is null skip/deny the action, otherwise call $user->getComplexPerm('structure')->hasStructurePerm(); mirror the same null-safe pattern used in var_imglist.php to ensure consistent, non-throwing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/css/list-widget.css`:
- Line 136: Replace the deprecated CSS declaration "word-break: break-word" with
the modern equivalent by removing that property and adding "overflow-wrap:
anywhere" (optionally keeping a fallback like "overflow-wrap: break-word" if
desired) wherever you find the "word-break: break-word" declaration so the
selector uses the semantically correct, MDN-recommended behavior.
In `@assets/js/customlink.js`:
- Around line 392-398: The hardcoded German title attributes in the new-item
HTML (created as variable $item using classes mform-cl-multi-handle and
mform-cl-multi-remove) should be replaced by values read from data-* attributes
injected server-side; modify the server-side widget to output data-title-move
and data-title-remove (using rex_i18n::msg('mform_cl_multi_move') /
'mform_cl_multi_remove'), then in customlink.js read those attributes from the
multi-widget container (e.g. container.dataset.titleMove / dataset.titleRemove)
and use them when building the $item string so tooltips are localized rather
than hardcoded.
In `@assets/js/list-widget.js`:
- Around line 281-282: Das aktuelle parts.forEach callback setzt das sichtbare
Label hart auf 'Artikel ' + value wenn type === 'linklist' (variable text), was
sprachabhängig ist; ändere die Logik in der Funktion (parts.forEach / die
Stelle, die die Variable text setzt) so dass es zuerst ein optionales
data-item-label Attribut vom Widget liest (z. B. dataset.itemLabel) und dieses
als Prefix verwendet, und falls nicht gesetzt alternativ die Artikelbezeichnung
über die REDAXO-API / rex_i18n auflöst; stelle sicher, dass die Frontend-Logik
nur benutzt wird, wenn ein lokaler Label-String vorhanden ist, andernfalls die
aufgelöste Artikelnamen-Fallback nutzt und vermeide das harte "Artikel " Prefix.
In `@lang/de_de.lang`:
- Around line 106-107: The two localization keys mform_widget_empty_media and
mform_widget_empty_entries use ASCII transliterations instead of German umlauts;
update their values to use proper umlauts by changing "Noch keine Medien
ausgewaehlt" to "Noch keine Medien ausgewählt" and "Keine Eintraege ausgewaehlt"
to "Keine Einträge ausgewählt" so they match the rest of the file's style.
In `@lang/es_es.lang`:
- Line 3: The translation key yform_values_custom_link_anchor in lang/es_es.lang
contains a missing accent: change the value from "Ocultar boton de anclaje" to
"Ocultar botón de anclaje" so it is consistent with other accented words in the
file; update the string for the yform_values_custom_link_anchor entry
accordingly.
In `@lang/fr_fr.lang`:
- Around line 1-3: Update the three French translation strings to include proper
accents and correct wording: change the value for mform_widget_empty_media to
"Aucun média sélectionné", change mform_widget_empty_entries to "Aucune entrée
sélectionnée", and review yform_values_custom_link_anchor and, if context
requires the noun form for "anchor", change its value to the typographically
preferred "Masquer le bouton d'ancrage" (otherwise keep "Masquer le bouton
d'ancre").
In `@lang/pt_br.lang`:
- Around line 1-3: Update the three translated values to include Portuguese
diacritics: change mform_widget_empty_media value to "Nenhuma mídia
selecionada", mform_widget_empty_entries value to "Nenhuma entrada selecionada"
(keep as-is if already correct) and change yform_values_custom_link_anchor value
to "Ocultar botão de âncora"; locate these keys (mform_widget_empty_media,
mform_widget_empty_entries, yform_values_custom_link_anchor) in the
lang/pt_br.lang file and replace the strings accordingly to match the accent
usage elsewhere.
In `@lang/sv_se.lang`:
- Line 3: The translation value for the key yform_values_custom_link_anchor is
missing the Swedish umlaut; update the string value from "Dolj ankarknappen" to
"Dölj ankarknappen" in lang/sv_se.lang (ensure the
yform_values_custom_link_anchor entry is edited) and save the file with UTF-8
encoding so diacritics are preserved.
In `@lib/MForm/FlexRepeater/MFormFlexRepeaterRenderer.php`:
- Around line 313-326: The toolbar buttons emitted in renderListWidget()
currently use <a href="#"> anchors which break keyboard behavior and
accessibility; update the markup generation in
MFormFlexRepeaterRenderer::renderListWidget() so each '<a href="#">' becomes a
'<button type="button">' preserving the existing classes (e.g. "btn btn-popup
mform-list-btn"), data-action attributes and icon contents, and add proper
accessibility attributes (use title or aria-label for the button label and
support disabled state via the disabled attribute when appropriate) so behavior
and A11y match the var_custom_linklist.php implementation.
- Line 36: Replace hardcoded German strings in MFormFlexRepeaterRenderer (e.g.,
the $btnText assignment and the item template / toolbar strings) with i18n
lookups using the existing language keys (mform_flex_repeater_add,
mform_flex_repeater_move_up, mform_flex_repeater_move_down,
mform_flex_repeater_add_after, mform_flex_repeater_toggle,
mform_flex_repeater_remove and the placeholder key for "wird im Flex-Repeater
derzeit nicht unterstuetzt"); update occurrences around the referenced blocks
(lines ~36, 181–197, 243–249, 313–326) to call the project’s translation helper
used elsewhere in the codebase (use the same translator function already in this
class or module) and pass the translated strings into the template/attributes
instead of the hardcoded German text so all locales use the proper i18n keys.
- Around line 127-138: In the switch handling widget types inside
MFormFlexRepeaterRenderer, there is an unreachable second return (a text-input
fallback) after the first return that calls self::wrapFormGroup($label,
self::renderUnsupportedWidgetPlaceholder($type), $item); remove the dead code
block that builds the sprintf() input (the second return and its sprintf wrapper
referencing $class, $key, $attrs, $item) so only the intended unsupported-widget
return remains; ensure no other logic depends on that removed fallback.
In `@lib/MForm/MFormElements.php`:
- Around line 224-237: The legacy addRepeaterElement currently stores $open in
$options['open'] but the frontend reads data-mfr-collapsed and
data-mfr-first-open; update addRepeaterElement to translate the legacy $open
boolean into the new attributes before calling addFlexRepeaterElement: set
options['collapsed'] = !$open and options['first_open'] = (bool)$open (and keep
options['open'] if you want backward compatibility), so existing calls using
addRepeaterElement(..., open: false) result in the repeater being initially
collapsed in flex-repeater.js; adjust in addRepeaterElement (which calls
addFlexRepeaterElement) and ensure MFormParser output will emit the
corresponding data-mfr-collapsed/data-mfr-first-open attributes.
In `@lib/Widget/var_custom_medialist.php`:
- Around line 77-95: The toolbar buttons currently render icons and rely only on
title attributes; update each button element (the variables/view: viewButton,
and buttons with data-action="open","add","view","up","down","delete") to
include an aria-label using the same localized strings from rex_i18n::msg(...)
so screenreaders get a reliable name, and for the view-toggle button
(constructed in viewButton with data-action="toggle-view") also add an
aria-pressed attribute that reflects the current state (true/false) and ensure
its value is kept in sync when the view is toggled.
In `@pages/module/extended/conditional_fields/output.inc`:
- Around line 39-41: The output.inc currently echoes raw $text (from
REX_VALUE[4]) causing an XSS risk; update the rendering in output.inc to escape
the user input by wrapping $text with rex_escape() and optionally apply nl2br()
if line breaks should become <br> tags so that $text is safely rendered (keep
input.inc as TextAreaField but ensure output uses rex_escape($text) or
nl2br(rex_escape($text)) where $text is output).
In `@pages/module/repeater/full_feature_lab/output.inc`:
- Around line 94-95: The code casts $row[$key] to string which triggers "Array
to string conversion" for fields like cta_links or downloads; update the logic
around $val assignment (the $val = ... line that references $row, $key) to
detect if $row[$key] is an array or Traversable and, instead of (string)
casting, normalize it to a readable string (e.g. json_encode($row[$key]) or
implode on relevant sub-values) and only cast scalars to string; ensure
htmlspecialchars is still used when echoing the value to avoid XSS.
In `@ytemplates/classic/value.custom_link.tpl.php`:
- Line 13: Die Zuweisung für 'anchor' ist invertiert: in
value.custom_link.tpl.php wird aktuell 'anchor' => (1 == (int)
$this->getElement('anchor')) ? 0 : 'enable' gesetzt, sodass bei aktivierter
Checkbox der Widget-Check ($args['anchor'] != 0 in
lib/Widget/var_custom_link.php) den Anker versteckt; ändere die Zuordnung so,
dass bei getElement('anchor') == 1 'enable' übergeben wird und sonst 0, damit
das Widget korrekten Verhalten zeigt.
---
Outside diff comments:
In `@lib/MForm/MFormElements.php`:
- Around line 333-402: The three methods addRadioImgField, addRadioIconField,
and addRadioColorField declare ?array $options = null but immediately foreach
over $options, causing a TypeError when null is passed; fix by making $options
default to an empty array (change signature to array $options = []) or add a
defensive early guard (e.g., if ($options === null) $options = []; then proceed)
and keep existing logic; update all three methods (addRadioImgField,
addRadioIconField, addRadioColorField) to use the same fix so foreach never
iterates over null.
- Around line 168-175: The addTabElement method calls addForm($form, $parse,
$showWrapper) which passes $showWrapper into the $debug parameter of addForm;
update the call in addTabElement so the third argument is false and the fourth
is $showWrapper (i.e. ->addForm($form, $parse, false, $showWrapper)) to match
the addForm signature and keep behavior consistent with addFieldsetArea,
addColumnElement, addInlineElement, addCollapseElement, and
addConditionalFieldsetArea.
---
Nitpick comments:
In `@assets/css/mform.css`:
- Around line 737-747: Die CSS-Regeln für .mform-cl-multi-remove verwenden
mehrere !important-Marker; stattdessen entferne die !important-Spezifizierer von
border, background, color und padding und steigere die Selektorspezifität (z. B.
.mform-cl-multi-item .mform-cl-multi-remove.btn oder ähnliche Kette) so dass die
Regeln gezielt generische .btn-Defaults überschreiben; passe dabei auch die
Hover-Regel (.mform-cl-multi-remove:hover) analog an (ohne !important) und
teste, dass Styling weiterhin greift und kein unerwünschtes Vererben auf andere
Buttons passiert.
In `@assets/js/customlink.js`:
- Around line 427-431: The mousedown handler registered on multiWidget
('mousedown.clmulti' for selector '.mform-cl-multi-handle') is dead code—either
remove the empty multiWidget.on('mousedown.clmulti', '.mform-cl-multi-handle',
...) handler entirely, or replace it with a real sortable integration:
initialize window.Sortable on the same multiWidget container (mirroring the
approach in assets/js/list-widget.js), wire the drag handle selector
'.mform-cl-multi-handle', and implement the move/swap logic in the Sortable
callbacks; prefer removal for minimal cleanup, or implement Sortable if you want
drag-to-reorder now.
In `@assets/js/flex-repeater.js`:
- Around line 319-323: Die aktuelle collectItemData-Logik sucht mit ':scope
.mfr-nested-repeater' und filtert per nested.closest('.mfr-item') !== itemEl,
was bei zusätzlicher Nesting-Tiefe falsch-positive Treffer erlauben kann; passe
den Selector in collectItemData so an, dass nur direkte Level‑2-Repeater
erwischt werden (z.B. ':scope > .mfr-item-body .mfr-nested-repeater' oder
kombiniere ':scope .mfr-nested-repeater:not(.mfr-nested-item *)') und
entferne/vereinfache das closest-Filter-Workaround; referenziere
collectItemData, collectNestedItems und die querySelectorAll-Callsite beim
Patch.
In `@assets/js/list-widget.js`:
- Around line 64-73: The short timeout (setTimeout(..., 2)) in the rex:ready
handler is being used as a deliberate micro-defer to let other rex:ready
handlers or async markup mounting complete before initializing list widgets; add
a clear comment immediately above this setTimeout explaining that intent
(mentioning the race with other rex:ready handlers/markup mounting and why a
minimal delay of 2ms was chosen) and reference the related symbols
mform_list_widget and mformListWidgetInit so future maintainers do not remove it
without validating ordering; optionally note that a safer alternative (e.g.,
explicit event or requestAnimationFrame) could be considered if refactoring.
- Around line 456-469: Replace the unreliable beforeunload handler in
mformListBindPopupSync with a polling loop that checks popup.closed: if popup is
falsy or callback is not a function keep early return; otherwise start a
setInterval (e.g. 100ms) that in a try/catch reads popup.closed and when true
clears the interval and invokes callback; in the catch branch (access errors)
treat as still-open until closed or clear interval and call callback if popup
reference becomes unusable, and ensure you clear the interval on callback to
avoid leaks.
In `@lib/Widget/var_custom_linklist.php`:
- Line 47: Replace the use of rex::requireUser() with a defensive null-checked
rex::getUser() call so the widget does not throw when no backend user exists;
specifically, in var_custom_linklist.php update the permission check that
currently calls
rex::requireUser()->getComplexPerm('structure')->hasStructurePerm() to instead
obtain $user = rex::getUser(); if $user is null skip/deny the action, otherwise
call $user->getComplexPerm('structure')->hasStructurePerm(); mirror the same
null-safe pattern used in var_imglist.php to ensure consistent, non-throwing
behavior.
In `@pages/module/extended/conditional_fields/output.inc`:
- Line 47: Der img src verwendet rex_url::media($image) ohne HTML-Escaping;
wrappe die URL-Ausgabe mit rex_escape() analog zur Iframe-Zeile (also
rex_escape(rex_url::media($image))) und behandle weiterhin $altText wie bisher;
aktualisiere die echo in pages/module/extended/conditional_fields/output.inc so
dass rex_url::media($image) vor dem Einfügen in src mittels rex_escape gesichert
wird, um Pfad-/Dateinamen-Sonderzeichen robust zu escapen.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a53584f-9def-4f82-b583-2948d2b0d302
📒 Files selected for processing (25)
assets/css/imglist.cssassets/css/list-widget.cssassets/css/mform.cssassets/js/customlink.jsassets/js/flex-repeater.jsassets/js/list-widget.jsboot.phplang/de_de.langlang/en_gb.langlang/es_es.langlang/fr_fr.langlang/pt_br.langlang/sv_se.langlib/MForm/FlexRepeater/MFormFlexRepeaterRenderer.phplib/MForm/MFormElements.phplib/Widget/var_custom_link.phplib/Widget/var_custom_linklist.phplib/Widget/var_custom_medialist.phplib/Widget/var_imglist.phppages/module/extended/conditional_fields/output.incpages/module/repeater/full_feature_lab/output.incytemplates/bootstrap/value.custom_link.tpl.phpytemplates/bootstrap/value.custom_link_multi.tpl.phpytemplates/classic/value.custom_link.tpl.phpytemplates/classic/value.custom_link_multi.tpl.php
🚧 Files skipped from review as they are similar to previous changes (2)
- ytemplates/bootstrap/value.custom_link_multi.tpl.php
- ytemplates/classic/value.custom_link_multi.tpl.php
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (4)
assets/css/list-widget.css (1)
133-141:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin –
word-break: break-wordist hier weiter deprecated.Stylelint meldet das an der Stelle zurecht. Für den neuen Grid-View bitte auf
overflow-wrap: anywhereumstellen, damit das Widget nicht mit veraltetem CSS startet.♻️ Vorschlag
.mform-list-widget.mform-list-widget-medialist.is-grid-view .mform-list-items li { white-space: normal; min-height: 48px; display: flex; flex-direction: column; align-items: flex-start; gap: .45rem; - word-break: break-word; + overflow-wrap: anywhere; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/css/list-widget.css` around lines 133 - 141, The rule using the deprecated property `word-break: break-word` in the selector `.mform-list-widget.mform-list-widget-medialist.is-grid-view .mform-list-items li` should be replaced with the modern equivalent: remove `word-break: break-word` and add `overflow-wrap: anywhere` so the grid-view list items use the non-deprecated wrapping behavior; update that declaration in the same selector block (keep the other properties intact).assets/js/list-widget.js (1)
319-321:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin – das Fallback-Label für Linklist-Items ist noch hartcodiert.
Artikellandet hier direkt im UI und ignoriert die Backend-Sprache. Bitte den Prefix aus einemdata-*-Attribut oder einem serverseitig aufgelösten Label ziehen.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/js/list-widget.js` around lines 319 - 321, Die harte Kodierung von "Artikel " in der parts.forEach-Schleife (wo text = type === 'linklist' ? ('Artikel ' + value) : value) muss entfernt werden; stattdessen lese das Prefix aus einem data-Attribut oder einem serverseitig aufgelösten Label (z.B. via $(container).data('linklist-prefix') oder container.dataset) und verwende dieses Prefix beim Aufbau von text in der bedingten Zuweisung (mit leerem Fallback, falls das data-Attribut nicht gesetzt ist); passe also die Stelle mit type === 'linklist' an, damit das option-Label das dynamische Prefix verwendet.lib/Widget/var_custom_medialist.php (1)
126-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMoin – die Icon-Buttons brauchen weiterhin echte Accessible Names.
titleallein reicht hier nicht. Ohnearia-labelsind Open/Add/View/Up/Down/Delete und der View-Toggle für Screenreader kaum nutzbar; beim Toggle fehlt zusätzlich ein sauber mitgepflegtesaria-pressed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/Widget/var_custom_medialist.php` around lines 126 - 145, Buttons rendered by the medialist widget (see the viewButton variable and the toolbar buttons with data-action="open", "add", "view", "up", "down", "delete" and the toggle at data-action="toggle-view") only use title attributes which is insufficient for screenreaders; add explicit accessible names by adding aria-label attributes to each button using the same localized strings (e.g. rex_i18n::msg('var_media_open'), 'var_media_new', 'var_media_view', 'var_medialist_move_up', 'var_medialist_move_down', 'var_media_remove' and the view labels used for the toggle), and for the toggle button (viewButton) also include a properly managed aria-pressed attribute that reflects the current view state (use $initialView or the toggle logic to set "true"/"false"); keep existing title attributes but ensure aria-label and aria-pressed are present and populated.assets/js/customlink.js (1)
423-428:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMoin – die Tooltips für neu hinzugefügte Multi-Items sind weiter hart auf Deutsch verdrahtet.
VerschiebenundEntfernenwerden hier bei dynamisch erzeugten Einträgen immer deutsch gesetzt. In mehrsprachigen Backends fallen damit genau die neu angelegten Items aus der Lokalisierung. Bitte die Titel wie beim Server-Markup ausdata-*-Attributen des Widgets lesen.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/js/customlink.js` around lines 423 - 428, The HTML for dynamically created multi-items currently hardcodes the titles "Verschieben" and "Entfernen"; change the $item creation to read these strings from the widget's data-* attributes instead: fetch the widget root (e.g., the element you render the multi-widget from or the element in scope where itemHtml is built), read data-mform-cl-multi-handle (or data-mform-cl-multi-handle-title) and data-mform-cl-multi-remove (or data-mform-cl-multi-remove-title) via jQuery .data(), and use those values when building the '<span class="mform-cl-multi-handle" title="...">' and the remove '<a ... title="...">' so newly added items use the localized titles like the server markup. Ensure you fall back to a default string if the data attribute is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/css/imglist.css`:
- Around line 4-9: Die angekündigte Rückwärtskompatibilität fehlt: entweder
stelle in assets/css/imglist.css einen kleinen CSS‑Shim wieder her, der die
alten Selektoren (.rex-js-widget-imglist, ul.thumbnail-list und evtl.
.thumbnail-list-item) auf die neuen Klassen (z.B. mappe zu .mform-list-widget,
.mform-list-widget-medialist, .is-grid-view bzw. die --mfl-* Variablen) abbildet
und damit grundlegende Darstellung (layout, spacing, thumbnails) sicherstellt,
oder dokumentiere in der Datei/README klar, dass die alten Selektoren entfernt
wurden und nenne explizit die neuen Klassen/Variablen (.mform-list-widget,
.mform-list-widget-medialist, --mfl-*) sowie die betroffene JS‑Initialisierung
(assets/js/imglist.js) als Breaking Change; wähle eine der beiden Optionen und
implementiere entweder den Shim oder die Dokumentationsänderung konsistent.
In `@assets/js/customlink.js`:
- Around line 68-74: The prompt and display labels (currently hardcoded/hybrid
German-English) must be loaded from server-provided translations instead of
literals; update promptValue and all callers to accept or fetch translated
strings from element data-* attributes or a global rex_i18n object injected
server-side (e.g., use element.dataset.promptLabel or window.rexI18n.linkLabel)
rather than embedding text in JS; specifically change promptValue usage and the
call sites referenced (around the existing promptValue function and the blocks
at the other locations) to read labels like "Link", "Mail", "Telephone",
"Anker-ID", "Anker:" from data attributes or rex_i18n and pass those into
promptValue so the UI text is consistent and localized.
In `@assets/js/list-widget.js`:
- Around line 333-337: The preview URL for direct media paths is built by
concatenating the raw filename, which breaks on spaces/special chars; update the
assignments that set option.attr('data-preview', '/media/' + value) (the
branches using isVideo, isSvg, isModernImage) to URL-encode the filename (use
the same approach as the raster branch that uses encodeURIComponent(value)), and
apply the same fix to the other occurrence around the 393–399 block so all
/media/ + filename concatenations use encodeURIComponent(value) consistently.
- Around line 101-103: The list items are focusable but only respond to mouse
clicks; add keyboard support by handling Enter and Space in the same way as the
click handler: attach a keydown (or keyup) handler for 'li' that calls
mformListSelect(widget, $(this).data('index')) when Enter (keyCode 13) or Space
(keyCode 32) is pressed, and ensure this is registered alongside the existing
click binding (the same selector as
list.off('click.mformListWidget').on('click.mformListWidget', 'li', ...)). Also
update the selection rendering to set an accessible state (e.g., toggle
aria-selected="true"/"false" on the selected li in the function that updates
selection—mformListSelect or its renderer) so screen readers see the selected
item; replicate the same changes for the other instance noted (lines ~371-375).
- Around line 170-175: Die Funktion mformListGetStoredView(...) ignoriert
momentan den gespeicherten Wert "gallery" (sie prüft nur 'grid' und 'list'),
wodurch mformListSetStoredView() zwar "gallery" speichern kann, der View-State
nach Reload aber nicht wiederhergestellt wird; ändere die Prüfbedingung in
mformListGetStoredView so dass sie auch 'gallery' als gültigen Rückgabewert
akzeptiert (z. B. return value === 'grid' || value === 'list' || value ===
'gallery' ? value : null), behalte die try/catch-Logik und Rückgabe von null für
ungültige Werte bei.
In `@lib/Widget/var_custom_linklist.php`:
- Around line 69-74: The toolbar buttons in var_custom_linklist.php render only
icons with title attributes, which is inaccessible; update each button element
(the ones with data-action="open", "up", "down", "delete") to include an
explicit aria-label using the same localized strings (e.g.,
rex_i18n::msg('var_link_open'), 'var_linklist_move_up',
'var_linklist_move_down', 'var_link_delete') and mark the <i> icon elements as
aria-hidden="true" so screenreaders announce the button label and ignore the
decorative icon; preserve existing title/disabled attributes ($disabled,
$disabledAria, $disabledClass) while adding these aria attributes.
---
Duplicate comments:
In `@assets/css/list-widget.css`:
- Around line 133-141: The rule using the deprecated property `word-break:
break-word` in the selector
`.mform-list-widget.mform-list-widget-medialist.is-grid-view .mform-list-items
li` should be replaced with the modern equivalent: remove `word-break:
break-word` and add `overflow-wrap: anywhere` so the grid-view list items use
the non-deprecated wrapping behavior; update that declaration in the same
selector block (keep the other properties intact).
In `@assets/js/customlink.js`:
- Around line 423-428: The HTML for dynamically created multi-items currently
hardcodes the titles "Verschieben" and "Entfernen"; change the $item creation to
read these strings from the widget's data-* attributes instead: fetch the widget
root (e.g., the element you render the multi-widget from or the element in scope
where itemHtml is built), read data-mform-cl-multi-handle (or
data-mform-cl-multi-handle-title) and data-mform-cl-multi-remove (or
data-mform-cl-multi-remove-title) via jQuery .data(), and use those values when
building the '<span class="mform-cl-multi-handle" title="...">' and the remove
'<a ... title="...">' so newly added items use the localized titles like the
server markup. Ensure you fall back to a default string if the data attribute is
missing.
In `@assets/js/list-widget.js`:
- Around line 319-321: Die harte Kodierung von "Artikel " in der
parts.forEach-Schleife (wo text = type === 'linklist' ? ('Artikel ' + value) :
value) muss entfernt werden; stattdessen lese das Prefix aus einem data-Attribut
oder einem serverseitig aufgelösten Label (z.B. via
$(container).data('linklist-prefix') oder container.dataset) und verwende dieses
Prefix beim Aufbau von text in der bedingten Zuweisung (mit leerem Fallback,
falls das data-Attribut nicht gesetzt ist); passe also die Stelle mit type ===
'linklist' an, damit das option-Label das dynamische Prefix verwendet.
In `@lib/Widget/var_custom_medialist.php`:
- Around line 126-145: Buttons rendered by the medialist widget (see the
viewButton variable and the toolbar buttons with data-action="open", "add",
"view", "up", "down", "delete" and the toggle at data-action="toggle-view") only
use title attributes which is insufficient for screenreaders; add explicit
accessible names by adding aria-label attributes to each button using the same
localized strings (e.g. rex_i18n::msg('var_media_open'), 'var_media_new',
'var_media_view', 'var_medialist_move_up', 'var_medialist_move_down',
'var_media_remove' and the view labels used for the toggle), and for the toggle
button (viewButton) also include a properly managed aria-pressed attribute that
reflects the current view state (use $initialView or the toggle logic to set
"true"/"false"); keep existing title attributes but ensure aria-label and
aria-pressed are present and populated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79921a20-5331-47d7-846a-f142828aa9cd
📒 Files selected for processing (8)
assets/css/imglist.cssassets/css/list-widget.cssassets/js/customlink.jsassets/js/imglist.jsassets/js/list-widget.jslib/Widget/var_custom_linklist.phplib/Widget/var_custom_medialist.phplib/Widget/var_imglist.php
This was referenced May 21, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Version 9.0.0
Breaking Change: MBlock-Support endet hier
view,view_switch)imagelist(statt reinem Dateityp-Text)rex_medialistbutton_preview)addCustomLinkMultipleField(...)fuer mehrere Custom-Links (repeater-basiert), Single-Format bleibt unveraendertaddConditionalFieldsetArea(...)für regelbasierte Anzeige von FormularbereichenMForm::registerTemplate($key, $class),MForm::fromTemplate($key)und->applyTemplate($key)ueber interne Registry (projektweite Defaults wiederverwendbar)copy_paste => trueamaddRepeaterElement()sessionStorage__disabled-Status wird nicht übernommencustom_link_multi– Mehrere Custom-Links in einem YForm-Feld (JSON-Array), identisches Widget wieaddCustomLinkMultipleField()custom_link– Anker-Button überanchor: 0ausblendbar; Bug im Classic-Template (extern→external) behobenMFormRepeaterHelper::decode()– bequemes Dekodieren von Repeater-Werten ohne Offline-ItemsSummary by CodeRabbit
Neue Funktionen
Verbesserungen
Bugfixes
Dokumentation