diff --git a/package.json b/package.json index 7fab85d6b..d90691c45 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "globals": "^17.7.0", "happy-dom": "^20.10.6", "headr": "^0.1.0", + "jquery": "^3.7.1", "npm-run-all2": "^9.0.0", "rimraf": "^6.1.3", "rollup": "^4.62.2", diff --git a/src/extensions/filter-control/bootstrap-table-filter-control.js b/src/extensions/filter-control/bootstrap-table-filter-control.js index cf6f37af3..00120da7c 100644 --- a/src/extensions/filter-control/bootstrap-table-filter-control.js +++ b/src/extensions/filter-control/bootstrap-table-filter-control.js @@ -478,13 +478,13 @@ $.BootstrapTable = class extends $.BootstrapTable { } // EVENTS - onColumnSearch ({ currentTarget, keyCode }) { + onColumnSearch ({ currentTarget, keyCode, isInitial }) { if (UtilsFilterControl.isKeyAllowed(keyCode)) { return } UtilsFilterControl.cacheValues(this) - const isInitialRender = !this._initialized + const isInitialRender = !this._initialized || isInitial === true // Cookie extension support if (!this.options.cookie) { @@ -548,16 +548,17 @@ $.BootstrapTable = class extends $.BootstrapTable { .html(`${Utils.sprintf(this.constants.html.icon, this.options.iconsPrefix, icon)} ${text}`) } - triggerSearch () { + triggerSearch (isInitial = false) { const searchControls = UtilsFilterControl.getSearchControls(this) searchControls.each(function () { const $element = $(this) + const eventName = $element.is('select') ? 'change' : 'keyup' - if ($element.is('select')) { - $element.trigger('change') + if (isInitial) { + $element.trigger(eventName, { isInitial: true }) } else { - $element.trigger('keyup') + $element.trigger(eventName) } }) } diff --git a/src/extensions/filter-control/utils.js b/src/extensions/filter-control/utils.js index f1cbccaf0..5fd7c5502 100644 --- a/src/extensions/filter-control/utils.js +++ b/src/extensions/filter-control/utils.js @@ -448,7 +448,8 @@ export function createControls (that, header) { if (addedFilterControl) { header.off('keyup', 'input').on('keyup', 'input', ({ currentTarget, keyCode }, obj) => { - keyCode = obj ? obj.keyCode : keyCode + keyCode = obj?.keyCode ?? keyCode + const isInitial = obj?.isInitial === true if (that.options.searchOnEnterKey && keyCode !== 13) { return @@ -466,13 +467,14 @@ export function createControls (that, header) { clearTimeout(currentTarget.timeoutId || 0) currentTarget.timeoutId = setTimeout(() => { - that.onColumnSearch({ currentTarget, keyCode }) + that.onColumnSearch({ currentTarget, keyCode, isInitial }) }, that.options.searchTimeOut) }) - header.off('change', 'select').on('change', 'select', ({ currentTarget, keyCode }) => { + header.off('change', 'select').on('change', 'select', ({ currentTarget, keyCode }, obj) => { const $selectControl = $(currentTarget) const value = $selectControl.val() + const isInitial = obj?.isInitial === true const normalizedValue = value === null ? $selectControl.prop('multiple') ? [] : null : @@ -482,7 +484,7 @@ export function createControls (that, header) { clearTimeout(currentTarget.timeoutId || 0) currentTarget.timeoutId = setTimeout(() => { - that.onColumnSearch({ currentTarget, keyCode }) + that.onColumnSearch({ currentTarget, keyCode, isInitial }) }, that.options.searchTimeOut) }) @@ -550,7 +552,7 @@ export function createControls (that, header) { } if (that.options.sidePagination !== 'server') { - that.triggerSearch() + that.triggerSearch(true) } if (!that.options.filterControlVisible) { diff --git a/tests/extensions/filter-control.test.js b/tests/extensions/filter-control.test.js new file mode 100644 index 000000000..9be248627 --- /dev/null +++ b/tests/extensions/filter-control.test.js @@ -0,0 +1,348 @@ +import { describe, expect, it, vi } from 'vitest' + +async function loadFilterControlPrototype () { + if (!globalThis.$) { + const { default: jq } = await import('jquery') + + globalThis.$ = jq + globalThis.jQuery = jq + } + await import('../../src/bootstrap-table.js') + await import('../../src/extensions/filter-control/bootstrap-table-filter-control.js') + + const $ = globalThis.jQuery || globalThis.$ + + expect($?.BootstrapTable).toBeTruthy() + + return { + $, + BootstrapTable: $.BootstrapTable + } +} + +describe('filter-control issue #8246', () => { + describe('runtime regression guards', () => { + it('triggerSearch() only attaches isInitial event data when requested', async () => { + const { $ } = await loadFilterControlPrototype() + + document.body.innerHTML = '
' + const $table = $('#ts-test') + + $table.bootstrapTable({ + filterControl: true, + columns: [{ field: 'a', title: 'A', filterControl: 'input' }], + data: [{ a: 'x' }] + }) + + const triggerSpy = vi.spyOn($.fn, 'trigger') + + try { + // triggerSearch(true) — every keyup/change must carry { isInitial: true } + triggerSpy.mockClear() + $table.bootstrapTable('triggerSearch', true) + + const initialEvents = triggerSpy.mock.calls.filter( + c => c[0] === 'keyup' || c[0] === 'change' + ) + + expect(initialEvents.length).toBeGreaterThan(0) + initialEvents.forEach(call => { + expect(call).toHaveLength(2) + expect(call[1]).toEqual({ isInitial: true }) + }) + + // triggerSearch() (no args) — must NOT attach extra event data, so the + // public API surface stays identical to the original signature. + triggerSpy.mockClear() + $table.bootstrapTable('triggerSearch') + + const userEvents = triggerSpy.mock.calls.filter( + c => c[0] === 'keyup' || c[0] === 'change' + ) + + expect(userEvents.length).toBeGreaterThan(0) + userEvents.forEach(call => { + expect(call).toHaveLength(1) + }) + } finally { + triggerSpy.mockRestore() + if ($table.data('bootstrap.table')) { + $table.bootstrapTable('destroy') + } + document.body.innerHTML = '' + } + }) + + it('only the initial onColumnSearch call carries isInitial: true; user input does not', async () => { + const { $, BootstrapTable } = await loadFilterControlPrototype() + const onColumnSearchSpy = vi.spyOn(BootstrapTable.prototype, 'onColumnSearch') + + document.body.innerHTML = '
' + const $table = $('#issue-8246-table') + + try { + $table.bootstrapTable({ + search: true, + filterControl: true, + searchTimeOut: 0, + columns: [{ field: 'name', title: 'Name', filterControl: 'input' }], + data: [{ name: 'alpha' }, { name: 'beta' }] + }) + + // Let the deferred onColumnSearch calls fired by the initial + // triggerSearch(true) drain. + await new Promise(resolve => setTimeout(resolve, 20)) + + const initialCalls = onColumnSearchSpy.mock.calls.slice() + const initialWithFlag = initialCalls.filter(c => c[0]?.isInitial === true) + + expect(initialWithFlag.length).toBeGreaterThan(0) + + // Now simulate a user keystroke and assert no subsequent + // onColumnSearch is flagged as initial. + onColumnSearchSpy.mockClear() + const $input = $table.closest('.bootstrap-table').find('thead input').first() + + expect($input.length).toBe(1) + + $input.val('alp') + $input.trigger('keyup') + + await new Promise(resolve => setTimeout(resolve, 20)) + + const userCalls = onColumnSearchSpy.mock.calls.slice() + + expect(userCalls.length).toBeGreaterThan(0) + userCalls.forEach(call => { + expect(call[0]?.isInitial).not.toBe(true) + }) + } finally { + onColumnSearchSpy.mockRestore() + if ($table.data('bootstrap.table')) { + $table.bootstrapTable('destroy') + } + document.body.innerHTML = '' + } + }) + }) + + describe('isInitialRender truth table', () => { + const computeIsInitialRender = (initialized, isInitial) => + !initialized || isInitial === true + + it('treats pre-initialization as initial render', () => { + expect(computeIsInitialRender(false, false)).toBe(true) + expect(computeIsInitialRender(false, undefined)).toBe(true) + }) + + it('treats the post-init triggerSearch(true) deferred call as initial render (the #8246 fix)', () => { + expect(computeIsInitialRender(true, true)).toBe(true) + }) + + it('treats user-initiated searches as not initial', () => { + expect(computeIsInitialRender(true, false)).toBe(false) + expect(computeIsInitialRender(true, undefined)).toBe(false) + }) + + it('only treats explicit isInitial === true as initial (defensive)', () => { + expect(computeIsInitialRender(true, 'yes')).toBe(false) + expect(computeIsInitialRender(true, 1)).toBe(false) + }) + }) + + describe('onColumnSearch effect simulation', () => { + function simulateOnColumnSearch (state) { + const ctx = { + _initialized: state._initialized, + _filterControlValuesLoaded: state._filterControlValuesLoaded ?? false, + options: { + cookie: state.cookie, + pageNumber: state.pageNumber + }, + onSearch: vi.fn() + } + + const isInitialRender = !ctx._initialized || state.isInitial === true + + if (!ctx.options.cookie) { + if (!isInitialRender) { + ctx.options.pageNumber = 1 + } + } else { + ctx._filterControlValuesLoaded = true + } + + ctx.onSearch({ currentTarget: null, firedByInitSearchText: isInitialRender }, false) + return ctx + } + + it('issue #8246: cookie-restored pageNumber survives the deferred initial-render call', () => { + const ctx = simulateOnColumnSearch({ + _initialized: true, + isInitial: true, + cookie: true, + pageNumber: 5 + }) + + expect(ctx.options.pageNumber).toBe(5) + expect(ctx._filterControlValuesLoaded).toBe(true) + expect(ctx.onSearch).toHaveBeenCalledWith( + expect.objectContaining({ firedByInitSearchText: true }), + false + ) + }) + + it('user-initiated cookie+filter search does NOT reset pageNumber inside filter-control, but signals core onSearch to reset', () => { + const ctx = simulateOnColumnSearch({ + _initialized: true, + isInitial: false, + cookie: true, + pageNumber: 5 + }) + + expect(ctx.options.pageNumber).toBe(5) + expect(ctx._filterControlValuesLoaded).toBe(true) + expect(ctx.onSearch).toHaveBeenCalledWith( + expect.objectContaining({ firedByInitSearchText: false }), + false + ) + }) + + it('non-cookie user search resets pageNumber to 1', () => { + const ctx = simulateOnColumnSearch({ + _initialized: true, + isInitial: false, + cookie: false, + pageNumber: 5 + }) + + expect(ctx.options.pageNumber).toBe(1) + expect(ctx.onSearch).toHaveBeenCalledWith( + expect.objectContaining({ firedByInitSearchText: false }), + false + ) + }) + + it('non-cookie initial render does not reset pageNumber', () => { + const ctx = simulateOnColumnSearch({ + _initialized: false, + isInitial: false, + cookie: false, + pageNumber: 5 + }) + + expect(ctx.options.pageNumber).toBe(5) + expect(ctx.onSearch).toHaveBeenCalledWith( + expect.objectContaining({ firedByInitSearchText: true }), + false + ) + }) + }) + + describe('cookie pageNumber persistence (the full reload-vs-filter chain)', () => { + function simulateOnColumnSearchToCookie (state) { + const setCookie = vi.fn() + const ctx = { + _initialized: state._initialized, + _filterControlValuesLoaded: false, + searchText: '', + options: { + cookie: true, + pageNumber: state.pageNumber, + search: false + } + } + + // -- filter-control.onColumnSearch (the cookie branch) -- + const isInitialRender = !ctx._initialized || state.isInitial === true + + if (!ctx.options.cookie) { + if (!isInitialRender) { + ctx.options.pageNumber = 1 + } + } else { + ctx._filterControlValuesLoaded = true + } + + // -- core.onSearch — the actual gate that historically resets pageNumber -- + const firedByInitSearchText = isInitialRender + + if (!firedByInitSearchText) { + ctx.options.pageNumber = 1 + } + + // -- cookie.onSearch override saves whatever options.pageNumber is now -- + setCookie('bs.table.pageNumber', ctx.options.pageNumber) + + return { ctx, setCookie } + } + + it('reload on page 2 keeps the cookie at 2 (issue #8246)', () => { + const { ctx, setCookie } = simulateOnColumnSearchToCookie({ + _initialized: true, + isInitial: true, + pageNumber: 2 + }) + + expect(ctx.options.pageNumber).toBe(2) + expect(setCookie).toHaveBeenCalledWith('bs.table.pageNumber', 2) + expect(setCookie).not.toHaveBeenCalledWith('bs.table.pageNumber', 1) + }) + + it('user filter on page 2 saves 1 to the cookie (expected behaviour)', () => { + const { ctx, setCookie } = simulateOnColumnSearchToCookie({ + _initialized: true, + isInitial: false, + pageNumber: 2 + }) + + expect(ctx.options.pageNumber).toBe(1) + expect(setCookie).toHaveBeenCalledWith('bs.table.pageNumber', 1) + }) + + it('reload on page 5 keeps the cookie at 5 (no off-by-one or hardcoded page)', () => { + const { ctx, setCookie } = simulateOnColumnSearchToCookie({ + _initialized: true, + isInitial: true, + pageNumber: 5 + }) + + expect(ctx.options.pageNumber).toBe(5) + expect(setCookie).toHaveBeenCalledWith('bs.table.pageNumber', 5) + }) + + it('reload on page 1 keeps the cookie at 1 (idempotent on first page)', () => { + const { ctx, setCookie } = simulateOnColumnSearchToCookie({ + _initialized: true, + isInitial: true, + pageNumber: 1 + }) + + expect(ctx.options.pageNumber).toBe(1) + expect(setCookie).toHaveBeenCalledWith('bs.table.pageNumber', 1) + }) + + it('two consecutive reloads each preserve their pageNumber (#8246 second-F5 regression)', () => { + // First reload from cookie = 2 + const first = simulateOnColumnSearchToCookie({ + _initialized: true, + isInitial: true, + pageNumber: 2 + }) + + expect(first.ctx.options.pageNumber).toBe(2) + expect(first.setCookie).toHaveBeenLastCalledWith('bs.table.pageNumber', 2) + + // Second reload reads what the first reload saved + const cookieAfterFirst = first.setCookie.mock.calls.at(-1)[1] + const second = simulateOnColumnSearchToCookie({ + _initialized: true, + isInitial: true, + pageNumber: cookieAfterFirst + }) + + expect(second.ctx.options.pageNumber).toBe(2) + expect(second.setCookie).toHaveBeenLastCalledWith('bs.table.pageNumber', 2) + }) + }) +}) diff --git a/yarn.lock b/yarn.lock index 04592de64..032e86f65 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4229,6 +4229,11 @@ isstream@~0.1.2: resolved "https://registry.yarnpkg.com/isstream/-/isstream-0.1.2.tgz#47e63f7af55afa6f92e1500e690eb8b8529c099a" integrity sha1-R+Y/evVa+m+S4VAOaQ64uFKcCZo= +jquery@^3.7.1: + version "3.7.1" + resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.7.1.tgz#083ef98927c9a6a74d05a6af02806566d16274de" + integrity sha512-m4avr8yL8kmFN8psrbFFFmB/If14iN5o9nw/NgnnM+kybDJpRsAynV2BsfpTYrTRysYUdADVD7CkUUizgkpLfg== + js-tokens@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-4.0.0.tgz#19203fb59991df98e3a287050d4647cdeaf32499"