From b28ac64c4a2717ff3c61bb5cdc9d4e126718d171 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Fri, 30 Oct 2026 09:51:30 -0700 Subject: [PATCH] fix(images): find the top modal by hit test and cover toasts across modals (#533) --- TESTING.md | 5 ++-- tests/browser/full-inspector.spec.mjs | 33 +++++++++++++++++++++ tests/js/prompt-blocks.test.js | 2 +- tests/js/workspace-images-selection.test.js | 1 + tests/test_image_reveal.py | 2 ++ utils/file_reveal.py | 1 + web/js/workspace/toast.mjs | 10 +++++-- 7 files changed, 49 insertions(+), 5 deletions(-) diff --git a/TESTING.md b/TESTING.md index 1a2d8bdf..15dffad5 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1573,8 +1573,9 @@ Recorded deviations on this page (spec ยง8): | Anything else (415, 404, 409, 500, offline) | "The folder could not be opened. Try again." | **D176 (#533, beyond the concept):** the viewer is a modal, so `showWorkspaceToast` - moves `#workspace-toast` into the topmost open modal, keeping its fixed position, - and back when that modal closes. Otherwise every toast raised from a modal is + moves `#workspace-toast` into the topmost open modal (by hit test when several are + open), keeping its fixed position, and back when that modal closes. A modal opened + after a toast covers it until the next toast. Otherwise every toast raised from a modal is covered and inert while `toHaveText` still passes: assert `toBeVisible` and the hit test as `full-inspector.spec.mjs` does. diff --git a/tests/browser/full-inspector.spec.mjs b/tests/browser/full-inspector.spec.mjs index ce3bea31..3e41b500 100644 --- a/tests/browser/full-inspector.spec.mjs +++ b/tests/browser/full-inspector.spec.mjs @@ -387,6 +387,39 @@ test('Show in folder asks the server to open the listed file and announces each await page.keyboard.press('Escape');await expect(page.locator('#viewer-dialog')).toHaveCount(0); await expect(toast).toBeVisible();await expect(page.locator('body > #workspace-toast')).toHaveCount(1); }); +// #533 D176: real modals in the real top layer, with the page's own toast module. +test('a toast stays on screen across nested, switched and closing modals',async({page,request})=>{ + await fullFixture(page,request); + await page.evaluate(async()=>{ + const url=performance.getEntriesByType('resource').map(entry=>entry.name).find(name=>name.includes('/toast.mjs')); + const {showWorkspaceToast}=await import(url); + window.say=showWorkspaceToast; + window.modal=(id,first=false)=>{const dialog=document.createElement('dialog');dialog.id=id;dialog.textContent=id;document.body[first?'prepend':'append'](dialog);dialog.showModal();return dialog;}; + window.onTop=()=>{const node=document.getElementById('workspace-toast');const box=node.getBoundingClientRect();return box.width>0&&document.elementFromPoint(box.x+box.width/2,box.y+box.height/2)===node;}; + }); + const parent=()=>page.evaluate(()=>{const host=document.getElementById('workspace-toast')?.parentElement;return host?host.id||host.localName:null;}); + const onTop=()=>page.evaluate(()=>window.onTop()); + await page.evaluate(()=>{window.modal('a');say('one');}); + expect(await parent()).toBe('a');expect(await onTop()).toBe(true); + // Nested: the top one holds it, even earlier in the page than the one under it. + await page.evaluate(()=>{window.modal('b',true);say('two');}); + expect(await parent()).toBe('b');expect(await onTop()).toBe(true); + // The top one closes while the toast shows: the message stays, in the modal now on top. + await page.evaluate(()=>document.getElementById('b').close()); + await expect.poll(parent).toBe('a');expect(await onTop()).toBe(true); + await expect(page.locator('#workspace-toast')).toHaveText('two'); + // Switched: the next modal takes the next toast. + await page.evaluate(()=>{document.getElementById('a').close();window.modal('c');say('three');}); + expect(await parent()).toBe('c');expect(await onTop()).toBe(true); + // Closed and removed at once, as the workspace's dialogs do: home, still showing. + await page.evaluate(()=>{const c=document.getElementById('c');c.close();c.remove();}); + await expect.poll(parent).toBe('body');expect(await onTop()).toBe(true); + await expect(page.locator('#workspace-toast')).toHaveText('three'); + // Removed while open, with no close event: the next message still shows. + await page.evaluate(()=>{window.modal('d');say('four');document.getElementById('d').remove();say('five');}); + expect(await parent()).toBe('body');expect(await onTop()).toBe(true); + await expect(page.locator('#workspace-toast')).toHaveText('five'); +}); test('full inspector shows six facts and scoped provenance',async({page,request},testInfo)=>{ await page.setViewportSize({width:1600,height:1100});await fullFixture(page,request);const panel=inspector(page); await expect(panel.getByRole('heading',{name:POSITIVE_TITLE,exact:true})).toBeVisible();await expect(panel.locator('.panel-heading .chip')).toHaveText('PNG');await expect(panel.locator('.detail-inner > p.small')).toHaveText('complete.png'); diff --git a/tests/js/prompt-blocks.test.js b/tests/js/prompt-blocks.test.js index ca96fe87..1b291f1a 100644 --- a/tests/js/prompt-blocks.test.js +++ b/tests/js/prompt-blocks.test.js @@ -13,7 +13,7 @@ function dom() { closest(selector){if(selector==='section'&&this.tagName==='section')return this;if(selector==='[data-send-role]'&&Object.hasOwn(this.dataset,'sendRole'))return this;if(selector==='[data-copy-role]'&&Object.hasOwn(this.dataset,'copyRole'))return this;if(selector==='[data-retry-prompts]'&&Object.hasOwn(this.dataset,'retryPrompts'))return this;return this.parent?.closest(selector);} click(target){this.listeners.get('click')?.({target});} } - const previous=global.document,toast=new Element('div');global.document={createElement:tag=>new Element(tag),createElementNS:(_,tag)=>new Element(tag),getElementById:id=>id==='workspace-toast'?toast:null}; + const previous=global.document,toast=new Element('div');global.document={querySelectorAll:()=>[],createElement:tag=>new Element(tag),createElementNS:(_,tag)=>new Element(tag),getElementById:id=>id==='workspace-toast'?toast:null}; const walk=(root,predicate)=>[...(predicate(root)?[root]:[]),...root.children.flatMap(child=>walk(child,predicate))]; return {Element,walk,toast,restore:()=>{global.document=previous;}}; } diff --git a/tests/js/workspace-images-selection.test.js b/tests/js/workspace-images-selection.test.js index 4590bc4c..0aa18166 100644 --- a/tests/js/workspace-images-selection.test.js +++ b/tests/js/workspace-images-selection.test.js @@ -69,6 +69,7 @@ function fakeDOM() { createElement: tag => new Element(tag), createTextNode: text => Object.assign(new Element('#text'), { _text: String(text) }), getElementById: id => (id === 'workspace-toast' ? toast : null), + querySelectorAll: () => [], }; return { Element, body, toast, restore() { globalThis.document = previous; } }; } diff --git a/tests/test_image_reveal.py b/tests/test_image_reveal.py index 5fa492f6..eff278e0 100644 --- a/tests/test_image_reveal.py +++ b/tests/test_image_reveal.py @@ -440,6 +440,7 @@ class TestRevealInFileManager(unittest.TestCase): "DESKTOP_SESSION": "plasma", "XDG_SESSION_DESKTOP": "KDE", "XDG_CONFIG_HOME": "/home/u/.config", + "XDG_DATA_HOME": "/home/u/.local/share", "XDG_DATA_DIRS": "/usr/share", # What xdg-open reads to choose an opener (kde-open6 on Plasma 6). "KDE_FULL_SESSION": "true", @@ -465,6 +466,7 @@ class TestRevealInFileManager(unittest.TestCase): "DESKTOP_SESSION": "plasma", "XDG_SESSION_DESKTOP": "KDE", "XDG_CONFIG_HOME": "/home/u/.config", + "XDG_DATA_HOME": "/home/u/.local/share", "XDG_DATA_DIRS": "/usr/share", "KDE_FULL_SESSION": "true", "KDE_SESSION_VERSION": "6", diff --git a/utils/file_reveal.py b/utils/file_reveal.py index dc0d5fbc..d7c61916 100644 --- a/utils/file_reveal.py +++ b/utils/file_reveal.py @@ -75,6 +75,7 @@ CHILD_ENV_NAMES = ( "XDG_DATA_DIRS", "XDG_CONFIG_DIRS", "XDG_CONFIG_HOME", + "XDG_DATA_HOME", # The X11 cookie: a non-default path under GDM or SDDM. "XAUTHORITY", "DBUS_SESSION_BUS_ADDRESS", diff --git a/web/js/workspace/toast.mjs b/web/js/workspace/toast.mjs index 2ec98216..8a530aaa 100644 --- a/web/js/workspace/toast.mjs +++ b/web/js/workspace/toast.mjs @@ -9,9 +9,15 @@ let releaseWait = null; let toastElement = null; let toastHome = null; -/** The open modal the user is in: the last in the page, as a nested dialog is opened after its parent. */ +/** + * The open modal the user is in. With several, the top layer decides, not page order (a viewer can sit earlier in the + * page than the dialog it opened from): only the top one can be hit, the rest being inert. + */ function topModal() { - return [...document.querySelectorAll('dialog')].filter(dialog => dialog.matches(':modal')).at(-1) ?? null; + const modals = [...document.querySelectorAll('dialog')].filter(dialog => dialog.matches(':modal')); + if (modals.length < 2) return modals[0] ?? null; + const hit = document.elementFromPoint?.(window.innerWidth / 2, window.innerHeight / 2)?.closest('dialog'); + return modals.includes(hit) ? hit : modals.at(-1); } /**