From a8d6b93eda93173849ffb3319a5d000611ff06f9 Mon Sep 17 00:00:00 2001 From: Derrick Austin Date: Tue, 7 Apr 2026 11:16:52 -0500 Subject: [PATCH 1/2] Fix infinite loop in form widget with circular dependsOn declarations When two or more form fields have circular dependsOn declarations (e.g., field A depends on B and B depends on A), the onRefreshDependants method triggers change events that cascade infinitely, freezing the browser tab. This fix adds cascade chain tracking through the jQuery event object. Each time a field triggers its dependants, the current field name is appended to a cascadeChain array on the event. Before refreshing, the method checks whether the triggering field already appears in the chain. If it does, a cycle has been detected and the cascade stops. This preserves legitimate transitive cascading (A -> B -> C) while preventing circular loops (A -> B -> A) of any depth. The event parameter was already being passed by jQuery as the third argument to the handler (after the two preset arguments from $.proxy), so no changes to the event binding are needed. Fixes #421 --- .../widgets/form/assets/js/winter.form.js | 28 +++- .../framework/FormWidgetDependants.test.js | 149 ++++++++++++++++++ .../js/fixtures/formWidget/FormWidgetStubs.js | 31 ++++ 3 files changed, 204 insertions(+), 4 deletions(-) create mode 100644 modules/system/tests/js/cases/framework/FormWidgetDependants.test.js create mode 100644 modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js diff --git a/modules/backend/widgets/form/assets/js/winter.form.js b/modules/backend/widgets/form/assets/js/winter.form.js index e85474603c..56f89a30b7 100644 --- a/modules/backend/widgets/form/assets/js/winter.form.js +++ b/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) diff --git a/modules/system/tests/js/cases/framework/FormWidgetDependants.test.js b/modules/system/tests/js/cases/framework/FormWidgetDependants.test.js new file mode 100644 index 0000000000..5fa8e1a133 --- /dev/null +++ b/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 += '
'; + } + + return '
' + + '
' + + fieldHtml + + '
' + + '
'; + } + + 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); + }); + }); +}); diff --git a/modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js b/modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js new file mode 100644 index 0000000000..64d5082d28 --- /dev/null +++ b/modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js @@ -0,0 +1,31 @@ +/* + * 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 synchronously invokes the success callback +jQuery.fn.request = function (handler, options) { + var deferred = jQuery.Deferred(); + deferred.success = function (fn) { + fn(); + return deferred; + }; + return deferred; +}; + +// $.fn.loadIndicator() - no-op stub +jQuery.fn.loadIndicator = function () { + return this; +}; From c55e92a412c7aeb675662051fdbc9a1b9a0a5e94 Mon Sep 17 00:00:00 2001 From: Derrick Austin Date: Tue, 7 Apr 2026 11:40:10 -0500 Subject: [PATCH 2/2] Use proper jQuery Deferred contract in request stub Updated the $.fn.request test stub to use jQuery.Deferred properly: - .success() is now an alias for .done() (matching the WinterCMS framework.js Request class contract) - Deferred is resolved immediately via deferred.resolve() - Exposes .done(), .fail(), .always() from the jQuery Deferred API --- .../js/fixtures/formWidget/FormWidgetStubs.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js b/modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js index 64d5082d28..03f88d04b8 100644 --- a/modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js +++ b/modules/system/tests/js/fixtures/formWidget/FormWidgetStubs.js @@ -15,13 +15,21 @@ jQuery.fn.render = function (fn) { fn(); }; -// $.fn.request() - stub that synchronously invokes the success callback +/* + * $.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) { - fn(); - return deferred; + return deferred.done(fn); }; + + deferred.resolve(); return deferred; };