Fix infinite loop in form widget with circular dependsOn declarations

modules/backend/widgets/form/assets/js/winter.form.js

@@ -164,13 +164,29 @@
 
     /*
      * Refresh a dependancy field
-     * Uses a throttle to prevent duplicate calls and click spamming
+     * Uses a throttle to prevent duplicate calls and click spamming.
+     *
+     * The event parameter is passed automatically by jQuery as the third
+     * argument (after the two preset arguments from $.proxy). It carries
+     * a cascadeChain array that tracks which fields have already been
+     * refreshed in the current cascade, preventing infinite loops when
+     * fields have circular dependsOn declarations.
      */
-    FormWidget.prototype.onRefreshDependants = function(fieldName, toRefresh) {
+    FormWidget.prototype.onRefreshDependants = function(fieldName, toRefresh, event) {
         var self = this,
             form = this.$el,
             formEl = this.$form,
-            fieldElements = this.getFieldElements()
+            fieldElements = this.getFieldElements(),
+            cascadeChain = (event && event.cascadeChain) || []
+
+        /*
+         * If this field already appears in the cascade chain, we have
+         * a circular dependency. Stop the cascade to prevent an
+         * infinite loop. See: https://github.com/wintercms/winter/issues/421
+         */
+        if (cascadeChain.indexOf(fieldName) !== -1) {
+            return
+        }
 
         if (this.dependantUpdateTimers[fieldName] !== undefined) {
             window.clearTimeout(this.dependantUpdateTimers[fieldName])
@@ -186,8 +202,12 @@
                 data: refreshData
             }).success(function() {
                 self.toggleEmptyTabs()
+
+                var newChain = cascadeChain.concat([fieldName])
                 $.each(toRefresh.fields, function(key, field) {
-                    $('[data-field-name="' + field + '"]').trigger('change')
+                    var cascadeEvent = $.Event('change')
+                    cascadeEvent.cascadeChain = newChain
+                    $('[data-field-name="' + field + '"]').trigger(cascadeEvent)
                 })
             })
         }, this.dependantUpdateInterval)

modules/system/tests/js/cases/framework/FormWidgetDependants.test.js

@@ -0,0 +1,149 @@
+import FakeDom from '../../helpers/FakeDom';
+
+jest.setTimeout(5000);
+
+describe('Form Widget dependsOn', function () {
+    /**
+     * Build a FakeDom with jQuery, WinterCMS foundation, stubs, and the form widget.
+     * The FormWidgetStubs fixture provides minimal implementations of ocJSON,
+     * $.fn.render, $.fn.request (synchronous success), and $.fn.loadIndicator.
+     */
+    function buildDom(html) {
+        return FakeDom
+            .new()
+            .addScript([
+                'modules/backend/assets/js/vendor/jquery.min.js',
+                'modules/system/assets/ui/js/foundation.baseclass.js',
+                'modules/system/assets/ui/js/foundation.controlutils.js',
+                'modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js',
+                'modules/backend/widgets/form/assets/js/winter.form.js',
+            ])
+            .render(html);
+    }
+
+    /**
+     * Build form HTML with fields and their dependsOn declarations.
+     *
+     * Collects all field names (both keys and referenced dependencies) and creates
+     * a div for each. Fields that have dependencies get data-field-depends attributes.
+     *
+     * @param {Object} fields - Map of field names to arrays of field names they depend on.
+     *                          e.g. { a: ['b'], b: ['a'] } for circular deps.
+     */
+    function buildFormHtml(fields) {
+        // Collect all unique field names (both dependents and their dependencies)
+        var allFields = {};
+        for (var name in fields) {
+            allFields[name] = fields[name];
+            fields[name].forEach(function (dep) {
+                if (!(dep in allFields)) {
+                    allFields[dep] = null;
+                }
+            });
+        }
+
+        var fieldHtml = '';
+        for (var fieldName in allFields) {
+            fieldHtml += '<div data-field-name="' + fieldName + '"';
+            if (allFields[fieldName] !== null) {
+                fieldHtml += " data-field-depends='" + JSON.stringify(allFields[fieldName]) + "'";
+            }
+            fieldHtml += '><input type="text" name="' + fieldName + '"></div>';
+        }
+
+        return '<form>'
+            + '<div data-control="formwidget" data-refresh-handler="onRefreshField">'
+            + fieldHtml
+            + '</div>'
+            + '</form>';
+    }
+
+    it('refreshes dependent fields when a field changes', function (done) {
+        buildDom(buildFormHtml({ fieldB: ['fieldA'] }))
+            .then(function (dom) {
+                var $ = dom.window.jQuery;
+                var requestSpy = jest.spyOn($.fn, 'request');
+
+                $('[data-field-name="fieldA"]').trigger('change');
+
+                // The form widget debounces with a 300ms timer
+                setTimeout(function () {
+                    try {
+                        expect(requestSpy).toHaveBeenCalledTimes(1);
+                        expect(requestSpy).toHaveBeenCalledWith(
+                            'onRefreshField',
+                            expect.objectContaining({
+                                data: expect.objectContaining({ fields: ['fieldB'] })
+                            })
+                        );
+                        done();
+                    } catch (e) {
+                        done(e);
+                    }
+                }, 500);
+            });
+    });
+
+    it('prevents infinite loop with circular dependsOn declarations', function (done) {
+        buildDom(buildFormHtml({ fieldA: ['fieldB'], fieldB: ['fieldA'] }))
+            .then(function (dom) {
+                var $ = dom.window.jQuery;
+                var requestSpy = jest.spyOn($.fn, 'request');
+
+                $('[data-field-name="fieldA"]').trigger('change');
+
+                // With 300ms debounce per step, an unguarded circular loop would fire
+                // many times in 2 seconds. The fix should limit this to exactly 2
+                // requests: A refreshes B, B refreshes A (blocked by cascade chain).
+                setTimeout(function () {
+                    try {
+                        expect(requestSpy.mock.calls.length).toBe(2);
+                        done();
+                    } catch (e) {
+                        done(e);
+                    }
+                }, 2000);
+            });
+    });
+
+    it('allows transitive cascading (A -> B -> C) without blocking', function (done) {
+        buildDom(buildFormHtml({ fieldB: ['fieldA'], fieldC: ['fieldB'] }))
+            .then(function (dom) {
+                var $ = dom.window.jQuery;
+                var requestSpy = jest.spyOn($.fn, 'request');
+
+                $('[data-field-name="fieldA"]').trigger('change');
+
+                // A->B (300ms) then B->C (600ms). Allow time for both.
+                setTimeout(function () {
+                    try {
+                        expect(requestSpy.mock.calls.length).toBe(2);
+                        done();
+                    } catch (e) {
+                        done(e);
+                    }
+                }, 1500);
+            });
+    });
+
+    it('stops cycle in a three-field circular chain (A -> B -> C -> A)', function (done) {
+        buildDom(buildFormHtml({ fieldA: ['fieldC'], fieldB: ['fieldA'], fieldC: ['fieldB'] }))
+            .then(function (dom) {
+                var $ = dom.window.jQuery;
+                var requestSpy = jest.spyOn($.fn, 'request');
+
+                $('[data-field-name="fieldA"]').trigger('change');
+
+                // A->B (300ms), B->C (600ms), C tries to refresh A but A is already
+                // in the cascade chain [fieldA, fieldB] so it stops. Total: 3 requests.
+                setTimeout(function () {
+                    try {
+                        expect(requestSpy.mock.calls.length).toBe(3);
+                        done();
+                    } catch (e) {
+                        done(e);
+                    }
+                }, 2000);
+            });
+    });
+});

modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js

@@ -0,0 +1,39 @@
+/*
+ * Stubs for testing winter.form.js in isolation.
+ *
+ * Provides minimal implementations of WinterCMS dependencies that the
+ * form widget relies on at load time.
+ */
+
+// ocJSON - used by paramToObj inside the form widget
+window.ocJSON = function (str) {
+    return JSON.parse(str);
+};
+
+// $(document).render() - used by the form widget to auto-initialize on DOM ready
+jQuery.fn.render = function (fn) {
+    fn();
+};
+
+/*
+ * $.fn.request() - stub that mimics WinterCMS's AJAX framework contract.
+ *
+ * Returns a resolved jQuery Deferred with .done()/.fail()/.always() and a
+ * .success() alias (matching the WinterCMS framework.js Request class).
+ * The deferred resolves immediately since there is no real network I/O.
+ */
+jQuery.fn.request = function (handler, options) {
+    var deferred = jQuery.Deferred();
+
+    deferred.success = function (fn) {
+        return deferred.done(fn);
+    };
+
+    deferred.resolve();
+    return deferred;
+};
+
+// $.fn.loadIndicator() - no-op stub
+jQuery.fn.loadIndicator = function () {
+    return this;
+};