From b28e5e12f513617d05e347a4be856be95fd589b6 Mon Sep 17 00:00:00 2001 From: Belisoful Date: Sat, 29 Aug 2026 20:25:16 +0000 Subject: [PATCH] Upgrades JS CallbackControl.onPostback - better logging on exception --- .../prado/activecontrols/activecontrols3.js | 32 ++++++-- .../pages/ActiveWebTemplateTest.page | 6 ++ .../js/activecontrols/activecontrols.test.js | 77 ++++++++++++++++++- ...tiveButtonPostBackFallbackTestCase.spec.js | 44 +++++++++++ 4 files changed, 152 insertions(+), 7 deletions(-) create mode 100644 tests/playwright/web/TActiveButtonPostBackFallbackTestCase.spec.js diff --git a/framework/Web/Javascripts/source/prado/activecontrols/activecontrols3.js b/framework/Web/Javascripts/source/prado/activecontrols/activecontrols3.js index fe3958b40..cb7b5130c 100644 --- a/framework/Web/Javascripts/source/prado/activecontrols/activecontrols3.js +++ b/framework/Web/Javascripts/source/prado/activecontrols/activecontrols3.js @@ -6,9 +6,20 @@ Prado.WebUI.CallbackControl = Prado.Class(Prado.WebUI.PostBackControl, { onPostBack(options, event) { - const request = new Prado.CallbackRequest(options.EventTarget, options); - request.dispatch(); - event.preventDefault(); + // Deliberate fallback: when dispatch() throws — typically from a + // ClientSide hook — the element's default action is NOT prevented, so a + // submit button degrades to a full-page postback instead of dead-ending + // the click. The error is logged and rethrown; note the request reaches + // the server without whatever the failed hook was preparing. + try { + const request = new Prado.CallbackRequest(options.EventTarget, options); + request.dispatch(); + event.preventDefault(); + } catch (e) { + if (typeof Logger != "undefined") + Logger.error("Callback dispatch failed; falling back to the default action", e.message); + throw e; + } } }); @@ -24,10 +35,19 @@ Prado.WebUI.TActiveLinkButton = Prado.Class(Prado.WebUI.CallbackControl); Prado.WebUI.TActiveImageButton = Prado.Class(Prado.WebUI.TImageButton, { onPostBack(options, event) { + // Deliberate fallback, as in CallbackControl. On failure the coordinate + // inputs are left in the form on purpose: the fallback full-page submit + // needs them to carry the click position. this.addXYInput(options, event); - const request = new Prado.CallbackRequest(options.EventTarget, options); - request.dispatch(); - event.preventDefault(); + try { + const request = new Prado.CallbackRequest(options.EventTarget, options); + request.dispatch(); + event.preventDefault(); + } catch (e) { + if (typeof Logger != "undefined") + Logger.error("Callback dispatch failed; falling back to the default action", e.message); + throw e; + } this.removeXYInput(options, event); } }); diff --git a/tests/harness/web/protected/pages/ActiveWebTemplateTest.page b/tests/harness/web/protected/pages/ActiveWebTemplateTest.page index a9d8621a0..64131dd9e 100644 --- a/tests/harness/web/protected/pages/ActiveWebTemplateTest.page +++ b/tests/harness/web/protected/pages/ActiveWebTemplateTest.page @@ -12,6 +12,12 @@ + + + diff --git a/tests/js/activecontrols/activecontrols.test.js b/tests/js/activecontrols/activecontrols.test.js index 0a183b868..4e94c515c 100644 --- a/tests/js/activecontrols/activecontrols.test.js +++ b/tests/js/activecontrols/activecontrols.test.js @@ -168,13 +168,51 @@ describe('TActiveButton', () => { expect(dispatchMock).toHaveBeenCalled(); }); - it('calls event.preventDefault() after dispatch', () => { + it('calls event.preventDefault()', () => { mockCallbackRequest(); const ctrl = new TActiveButton({ ID: 'btn1', EventTarget: 'btn1' }); const evt = fakeEvent({ target: btn }); ctrl.onPostBack({ EventTarget: 'btn1' }, evt); expect(evt.preventDefault).toHaveBeenCalled(); }); + + it('does not prevent the default when dispatch throws, so the postback fallback runs', () => { + // Deliberate v4.4 design: a throwing ClientSide hook degrades the click + // to a full-page postback rather than dead-ending it. The error is + // rethrown so it stays observable. + const { dispatchMock } = mockCallbackRequest(); + dispatchMock.mockImplementation(() => { + throw new Error('ClientSide hook failed'); + }); + const ctrl = new TActiveButton({ ID: 'btn1', EventTarget: 'btn1' }); + const evt = fakeEvent({ target: btn }); + expect(() => ctrl.onPostBack({ EventTarget: 'btn1' }, evt)).toThrow('ClientSide hook failed'); + expect(evt.preventDefault).not.toHaveBeenCalled(); + }); + + it('logs the dispatch failure before rethrowing, when a Logger is present', () => { + // Skipping preventDefault() on a throw predates the fallback being + // deliberate, because it already sat after dispatch(). The log line is + // what the v4.4 try/catch adds, so it is what pins the change. + const { dispatchMock } = mockCallbackRequest(); + dispatchMock.mockImplementation(() => { + throw new Error('ClientSide hook failed'); + }); + const errorMock = vi.fn(); + global.Logger = { error: errorMock }; + + try { + const ctrl = new TActiveButton({ ID: 'btn1', EventTarget: 'btn1' }); + const evt = fakeEvent({ target: btn }); + expect(() => ctrl.onPostBack({ EventTarget: 'btn1' }, evt)).toThrow('ClientSide hook failed'); + expect(errorMock).toHaveBeenCalledWith( + 'Callback dispatch failed; falling back to the default action', + 'ClientSide hook failed', + ); + } finally { + delete global.Logger; + } + }); }); // ─── TActiveLinkButton ──────────────────────────────────────────────────────── @@ -277,6 +315,43 @@ describe('TActiveImageButton', () => { expect(form.querySelector('#img1_x')).toBeNull(); expect(form.querySelector('#img1_y')).toBeNull(); }); + + it('leaves the default action and the x/y inputs in place when dispatch throws', () => { + // Deliberate v4.4 design: the fallback full-page submit proceeds and + // needs the coordinate inputs to carry the click position. + const { dispatchMock } = mockCallbackRequest(); + dispatchMock.mockImplementation(() => { + throw new Error('ClientSide hook failed'); + }); + const ctrl = new TActiveImageButton({ ID: 'img1', EventTarget: 'img1' }); + const evt = fakeEvent({ target: img, clientX: 5, clientY: 5 }); + + expect(() => ctrl.onPostBack({ EventTarget: 'img1' }, evt)).toThrow('ClientSide hook failed'); + expect(evt.preventDefault).not.toHaveBeenCalled(); + expect(form.querySelector('#img1_x')).not.toBeNull(); + expect(form.querySelector('#img1_y')).not.toBeNull(); + }); + + it('logs the dispatch failure before rethrowing, when a Logger is present', () => { + const { dispatchMock } = mockCallbackRequest(); + dispatchMock.mockImplementation(() => { + throw new Error('ClientSide hook failed'); + }); + const errorMock = vi.fn(); + global.Logger = { error: errorMock }; + + try { + const ctrl = new TActiveImageButton({ ID: 'img1', EventTarget: 'img1' }); + const evt = fakeEvent({ target: img, clientX: 5, clientY: 5 }); + expect(() => ctrl.onPostBack({ EventTarget: 'img1' }, evt)).toThrow('ClientSide hook failed'); + expect(errorMock).toHaveBeenCalledWith( + 'Callback dispatch failed; falling back to the default action', + 'ClientSide hook failed', + ); + } finally { + delete global.Logger; + } + }); }); // ─── TActiveCheckBox ───────────────────────────────────────────────────────── diff --git a/tests/playwright/web/TActiveButtonPostBackFallbackTestCase.spec.js b/tests/playwright/web/TActiveButtonPostBackFallbackTestCase.spec.js new file mode 100644 index 000000000..2a6f9d362 --- /dev/null +++ b/tests/playwright/web/TActiveButtonPostBackFallbackTestCase.spec.js @@ -0,0 +1,44 @@ +import { test, expect } from '@playwright/test'; +import { PradoTestHelper, GENERIC_BASE_URL } from '../helpers.js'; + +const PAGE_URL = 'web/index.php?page=ActiveWebTemplateTest'; + +/** + * Deliberate v4.4 design: a ClientSide hook that throws lets the submit button + * fall back to a full-page postback, so the click does not dead-end. The error + * is rethrown and observable before the navigation. + * + * Prado.WebUI.CallbackControl.onPostBack() reaches event.preventDefault() only + * after dispatch() returns, and dispatch() runs the OnPreDispatch hook + * synchronously, so a throwing hook leaves the button's native submit in place. + * The unit-level counterpart is in tests/js/activecontrols/activecontrols.test.js. + */ +test('a throwing ClientSide hook falls back to a full-page postback', async ({ page }) => { + const h = new PradoTestHelper(page, GENERIC_BASE_URL); + await h.url(PAGE_URL); + + await page.locator('#ctl0_Content_btnStamp').click(); + await h.waitForAjaxCalls(); + await expect(page.locator('#listBody .row')).toHaveCount(1); + + const errors = []; + page.on('pageerror', (e) => errors.push(e.message)); + let navigations = 0; + page.on('framenavigated', (f) => { + if (f === page.mainFrame()) { + navigations++; + } + }); + + await page.locator('#ctl0_Content_btnThrowingHook').click(); + await page.waitForLoadState('domcontentloaded'); + await page.waitForTimeout(300); + + // The hook's error surfaced before the navigation wiped the page + expect(errors.join(' ')).toContain('deliberate hook failure'); + // and the click degraded to a full-page postback + expect(navigations).toBe(1); + // which re-rendered the page, discarding the stamped copy (PersistInstances + // is not enabled on this page) + await expect(page.locator('#listBody .row')).toHaveCount(0); +});