Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
});

Expand All @@ -24,10 +35,19 @@
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);
}
});
Expand Down Expand Up @@ -425,7 +445,7 @@
}
},

onToggle : function(options, event)

Check warning on line 448 in framework/Web/Javascripts/source/prado/activecontrols/activecontrols3.js

View workflow job for this annotation

GitHub Actions / Prado JS (ESLint + Vitest)

'event' is defined but never used. Allowed unused args must match /^_/u
{
if (this._suppressToggle) {
this._suppressToggle = false;
Expand Down Expand Up @@ -508,7 +528,7 @@
}
},

onDialogClose : function(options, event)

Check warning on line 531 in framework/Web/Javascripts/source/prado/activecontrols/activecontrols3.js

View workflow job for this annotation

GitHub Actions / Prado JS (ESLint + Vitest)

'event' is defined but never used. Allowed unused args must match /^_/u
{
if (this._suppressClose) {
this._suppressClose = false;
Expand Down
6 changes: 6 additions & 0 deletions tests/harness/web/protected/pages/ActiveWebTemplateTest.page
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
<com:TActiveButton ID="btnUpdateAll" Text="Update All" OnClick="server_update_all" />
<com:TActiveButton ID="btnSetContent" Text="Set Content" OnClick="server_set_content" />

<!-- The ClientSide hook throws, so the callback dispatch fails and the click
degrades to a full-page postback. It carries no OnClick handler: the
fallback request re-renders the page and nothing is stamped. -->
<com:TActiveButton ID="btnThrowingHook" Text="Throwing Hook"
ClientSide.OnPreDispatch="throw new Error('deliberate hook failure');" />

<!-- The UID is allocated on the client, so these send it as the callback
parameter. TCallback delivers it as a TCallbackEventParameter. -->
<com:TCallback ID="cbUpdateFirst" OnCallback="server_update_first" />
Expand Down
77 changes: 76 additions & 1 deletion tests/js/activecontrols/activecontrols.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ────────────────────────────────────────────────────────
Expand Down Expand Up @@ -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 ─────────────────────────────────────────────────────────
Expand Down
44 changes: 44 additions & 0 deletions tests/playwright/web/TActiveButtonPostBackFallbackTestCase.spec.js
Original file line number Diff line number Diff line change
@@ -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);
});
Loading