From f29f985386b77eb7592307874353d7a832d54e35 Mon Sep 17 00:00:00 2001 From: kolaente Date: Sat, 11 Apr 2026 18:11:02 +0200 Subject: [PATCH] test(modal): cover open race for #2590 --- frontend/src/components/misc/Modal.test.ts | 156 +++++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 frontend/src/components/misc/Modal.test.ts diff --git a/frontend/src/components/misc/Modal.test.ts b/frontend/src/components/misc/Modal.test.ts new file mode 100644 index 000000000..315c7bb96 --- /dev/null +++ b/frontend/src/components/misc/Modal.test.ts @@ -0,0 +1,156 @@ +import {describe, it, expect, beforeEach, afterEach, vi} from 'vitest' +import {mount, flushPromises} from '@vue/test-utils' +import {nextTick} from 'vue' +import Modal from './Modal.vue' + +// jsdom does not implement HTMLDialogElement.showModal/close. +// Provide stubs so that the [open] attribute — which CSS and our tests +// check — is flipped the same way the real browser would. +let showModalSpy: ReturnType +let closeSpy: ReturnType +let installedShowModal = false +let installedClose = false + +beforeEach(() => { + const proto = HTMLDialogElement.prototype + if (typeof proto.showModal !== 'function') { + proto.showModal = function () {} + installedShowModal = true + } + if (typeof proto.close !== 'function') { + proto.close = function () {} + installedClose = true + } + showModalSpy = vi.spyOn(proto, 'showModal').mockImplementation(function (this: HTMLDialogElement) { + this.setAttribute('open', '') + }) + closeSpy = vi.spyOn(proto, 'close').mockImplementation(function (this: HTMLDialogElement) { + this.removeAttribute('open') + }) +}) + +afterEach(() => { + showModalSpy.mockRestore() + closeSpy.mockRestore() + // Remove the prototype stubs we installed, so other test files see the + // original (unpatched) shape of HTMLDialogElement. + if (installedShowModal) { + // @ts-expect-error — removing the method we added + delete HTMLDialogElement.prototype.showModal + installedShowModal = false + } + if (installedClose) { + // @ts-expect-error — removing the method we added + delete HTMLDialogElement.prototype.close + installedClose = false + } + document.body.innerHTML = '' +}) + +describe('Modal.vue — open race condition (#2590)', () => { + it('opens the dialog when enabled flips false → true', async () => { + const wrapper = mount(Modal, { + attachTo: document.body, + props: {enabled: false}, + slots: {default: '

hi

'}, + }) + + // Pre-condition: dialog is not yet in the DOM. + expect(document.querySelector('dialog.modal-dialog')).toBeNull() + + // Flip enabled → true. This is the failure path in the bug report. + // The fix must call showModal() deterministically — i.e. once the + // element is mounted via the dialogRef watcher, not via a + // nextTick that may fire before the mount flush under Electron. + await wrapper.setProps({enabled: true}) + await flushPromises() + await nextTick() + + const dialog = document.querySelector('dialog.modal-dialog') as HTMLDialogElement | null + expect(dialog).not.toBeNull() + expect(dialog!.hasAttribute('open')).toBe(true) + expect(showModalSpy).toHaveBeenCalledTimes(1) + + wrapper.unmount() + }) + + it('calls showModal synchronously with the render flush, not via a deferred nextTick (#2590)', async () => { + // Regression guard: the buggy implementation scheduled showModal() via + // nextTick *after* setting showDialog = true, so the call landed in a + // microtask that could fire before the mount flush under + // Electron/Chromium. The fix invokes showModal() from a watch on the + // dialogRef template ref, which Vue populates during the same flush + // that mounts the element. That means by the time `await nextTick()` + // resolves after the first state change, the dialog must already have + // [open] set — no additional flushPromises or extra ticks required. + const wrapper = mount(Modal, { + attachTo: document.body, + props: {enabled: false}, + slots: {default: '

hi

'}, + }) + expect(document.querySelector('dialog.modal-dialog')).toBeNull() + + // Flip enabled and wait exactly one render flush. After this, the + // dialog is mounted AND showModal has been called. + wrapper.setProps({enabled: true}) + await nextTick() + + const dialog = document.querySelector('dialog.modal-dialog') as HTMLDialogElement | null + expect(dialog).not.toBeNull() + expect(showModalSpy).toHaveBeenCalled() + expect(showModalSpy.mock.instances[0]).toBe(dialog) + expect(dialog!.hasAttribute('open')).toBe(true) + + wrapper.unmount() + }) + + it('calls showModal on the exact dialog element that is mounted (race regression)', async () => { + // This test asserts the fix's contract: whenever the element + // is mounted (i.e. dialogRef becomes non-null), showModal() is called + // on *that* element. The buggy implementation instead relied on a + // nextTick callback whose timing could fire before the dialog mounted, + // skipping the showModal() call entirely and leaving .open === false. + const wrapper = mount(Modal, { + attachTo: document.body, + props: {enabled: true}, + slots: {default: '

hi

'}, + }) + await flushPromises() + await nextTick() + + const dialog = document.querySelector('dialog.modal-dialog') as HTMLDialogElement | null + expect(dialog).not.toBeNull() + // The fingerprint from the bug report: element is mounted but .open + // is false because showModal() was never called. The fix guarantees + // these two always agree. + expect(dialog!.hasAttribute('open')).toBe(true) + expect(showModalSpy).toHaveBeenCalled() + expect(showModalSpy.mock.instances[0]).toBe(dialog) + + wrapper.unmount() + }) + + it('closes the dialog when enabled flips true → false', async () => { + const wrapper = mount(Modal, { + attachTo: document.body, + props: {enabled: true}, + slots: {default: '

hi

'}, + }) + await flushPromises() + await nextTick() + + // Sanity: open. + expect(document.querySelector('dialog.modal-dialog')?.hasAttribute('open')).toBe(true) + + await wrapper.setProps({enabled: false}) + // Wait past the 150ms closeTimer (real timers — fake timers interact + // badly with Vue's scheduler). + await new Promise(resolve => setTimeout(resolve, 200)) + await flushPromises() + await nextTick() + + expect(document.querySelector('dialog.modal-dialog')).toBeNull() + + wrapper.unmount() + }) +})