Skip to content

Issue 226 Note Image Could Not Be Closed

Ed Mozley edited this page Oct 28, 2026 · 4 revisions

A picture on a note could not be closed

Reported: #226 by Dr4inder, October 2026 Β· Module: Tickets Β· Present in: every version since files on notes arrived (discussion #69) Β· Fixed in: 4.4.0


What you saw

Open a ticket, click a picture attached to one of its notes, and the picture fills the browser. There is no close button, Escape does nothing, and the browser's Back button leaves the ticket rather than returning to the note.


What was actually wrong

A file on a note was drawn as a plain link to the document download address:

const href = '../api/documents/download.php?id=' + encodeURIComponent(d.id);
return `<a class="note-file" href="${escapeHtml(href)}" ...>`;

No target, no click handler. For most files that is fine - the server sends them as a download, and the page stays where it is. A picture is different: a picture is safe to show in the browser, so the server sends it to be shown, not saved. The link then did what any link does - it navigated the whole Tickets page away and replaced it with the bare image.

That explains all three symptoms at once:

  • No close button - there is no page any more, only the browser's own image display.
  • Escape does nothing - the browser's image display has no Escape action.
  • Back leaves the ticket - the Tickets page opens a ticket without changing the address, so the history entry before the picture is the inbox, not the ticket.

πŸ”‘ Email attachments never had this problem, because they open with window.open(..., '_blank') - a new tab. The note files, added later, used a different pattern. It was the only link to download.php anywhere in the app.


The wrong turn

The one-line fix was target="_blank", the same as email attachments. It stops the page being replaced, but it is not what was asked for: the picture lands in another tab, with the ticket out of sight, and "close" means closing a tab. The reporter asked for a close control and Escape.

What was built instead: the picture opens in a floating window over the ticket, the same one the Files module uses - drag it, resize it from any edge or corner, snap it, maximise it, zoom, download, close with Γ— or Escape.


The fix: share the Files window, don't copy it

Files already had a window that does all of that, and a viewer that shows pictures, PDFs and Office files. But neither could simply be loaded on the Tickets page:

  • The window manager (files-wm.js) reached for Files-only parts as soon as it loaded - the desktop area, the taskbar, the hover previews, the auto-hiding navbar. On the Tickets page those do not exist.
  • The viewer (files-viewer.js) fetched its file through Files' own access tokens. A note's file is a different kind of document, with its own permission check.

So the window was split in two:

  1. assets/js/float-window.js - the window itself: drag, resize, snap, maximise, focus. Works on any page. FloatWin.page() gives an ordinary page its own window layer; Escape closes the window in front.
  2. files-wm.js keeps the desktop half (taskbar, minimise, previews, menus) and builds on the shared window through hooks. The Files desktop behaves exactly as before.
  3. The viewer gained FilesViewer.openUrl(): open a file from an address the caller already has. Tickets passes the note file's own download.php address, so the permission check is the same one the link always had.

Details for developers: Floating windows and the file viewer.

Small things that mattered

  • The Download button - the viewer's Download made a link to the same address, which serves a picture to show. Without the download attribute it would have opened the picture in the tab - the very bug being fixed. openUrl sets it.
  • Escape - the window takes focus when it opens, so Escape works straight away. But Escape on a page full of text boxes is also "cancel what I'm typing", so a window is left alone while the person is typing in a field elsewhere on the page.
  • Ctrl-click and middle-click still open the picture in a new tab - the link stays a real link, and only a plain click is intercepted.
  • SVG is not opened in the viewer - it can carry script. It stays a plain link.
  • Phones get the Files phone viewer: full screen, with Back.

πŸ“ Files changed

File Change
🟩 assets/js/float-window.js new - the window, moved out of files-wm.js
🟩 assets/css/float-window.css new - the window and viewer styles, moved out of files.css unchanged
🟦 assets/js/files-wm.js builds on the shared window; taskbar via hooks
🟦 assets/js/files-viewer.js openUrl(); works without FILES_BOOT / FilesWM
🟦 assets/js/inbox.js a click on a note's picture opens it in the viewer
🟦 assets/css/mobile.css the phone viewer is no longer Files-only
🟨 tickets/index.php, files/index.php load the shared files; Tickets exports only the viewer's strings

How it was proved

In headless Chrome, signed in, against a real ticket with a picture on a note:

Check Result
Click the picture a window opens over the ticket; the page address does not change
The picture loaded, fitted to the window (52%), zoom and fit buttons present
Maximise, then restore fills the screen, then back to its size
Drag the title bar 150, 100 moved 150, 100
Drag the bottom-right corner in 200, 100 shrank 200, 100
Download a link to the same address with download set to the file name
Click the same picture again the window comes to the front - still one window
Ctrl-click left to the browser (new tab)
Escape the window closes
Phone width (390px) full-screen viewer with Back; Back closes it

The control: with the old inbox.js the same test fails - there is no picture link to open.

Files, re-checked - because its window code moved: a window opens with its taskbar button; minimise, restore from the taskbar, a taskbar click on the front window minimises it; renaming a window renames its button; maximise reports the geometry for saving; focus moves between two windows' buttons; closing removes the button; the viewer opens a real picture from Files.

Not proven

  • Drag, resize and snap with a real mouse. The test drove them with synthetic pointer events, which exercise the same code - but nobody has yet dragged one by hand on the Tickets page.
  • The other viewer kinds on Tickets (PDF, Word, spreadsheets) - openUrl can show them, but Tickets only uses it for pictures, as asked.

Follow-up: email attachments, refitting and PDFs (also 4.4.0)

Asked for straight after the first fix:

  • Pictures attached to emails open the same way. The ticket's Attachments list - its thumbnails, its rows, and the phone's attachment cards - used window.open(..., '_blank'). openAttachment() now looks the attachment up, and a picture goes to FilesViewer.openUrl() with the same get_attachment.php address (same permission check); anything else still opens a new tab. The window opens above the Attachments list, and Escape closes the window, not the list.
  • The picture refits as the window is resized, as a PDF already did - until you zoom in or out yourself; Fit hands it back. This changes the Files viewer too: Files said pictures kept fitting, and until now they did not.

The trap in the refit

The obvious code - a ResizeObserver on the stage that resizes the picture inside its callback - logged "ResizeObserver loop completed with undelivered notifications". Resizing the picture adds or removes the stage's scrollbars, which resizes the stage again inside the same callback; the browser drops the notifications it cannot deliver, and the dropped ones were real window resizes that never refitted. The refit now runs just after the callback (a 16ms timer, as the PDF view uses 120ms), never inside it.

How it was proved - and why the first test lied

Headless Chrome with a virtual clock (--virtual-time-budget) does not lay the page out between script steps, so a resize observer fires or not depending on whether the test happens to read a size. The same build passed, failed, and half-passed on three runs. It was re-run in real time - headless Chrome kept alive with --remote-debugging-port and stopped after 40 seconds, reading the console live:

Step Picture width
Opened in an 820 Γ— 600 window 652px (16%)
Window shrunk to 500 Γ— 400 385px
Maximised 1055px
Restored 385px
Zoom in, then resize 482px β†’ 482px - left alone
Fit, then resize 719px β†’ 452px - refitting again

Control: the same run with the previous viewer - 632px at every step.

Also checked: a thumbnail and a row each open a window (no new tab); a second picture opens a second window in front; Escape closes the front window and leaves the Attachments list open; a PDF attachment still opens a new tab.

PDFs too (4.4.0)

Ed then asked for PDFs as well. One list in inbox.js, VIEWER_FILE_RE, says what Tickets sends to the viewer - pictures and PDFs - for both note files and email attachments. The viewer can show more (Word, spreadsheets, text, zips); those were left as downloads by choice, and zips would need a guard first, because opening something inside a zip writes to Files' audit log by Files id.

The trap it found: outside Files the viewer's base path was '../'. pdf.js loads with import(), and import() resolves a relative path against the script's address (assets/js/), not the page's - so it would have looked in assets/assets/js/vendor/. Pictures never noticed, because an <img> resolves against the page. The base is now worked out from the viewer's own script URL when Files is not there to supply BASE_URL.

Proved in real time on two real tickets: a 12-page PDF on a note and a 2-page PDF emailed in each open in a window (no new tab), draw their pages, show "1 of 12" / "1 of 2" and zoom, and the note's PDF refits when the window shrinks (page 762px β†’ 502px). The note's picture still opens beside it. No errors.


What this means for you

  • After upgrading to 4.4.0, hard-refresh (Ctrl-F5). Clicking a picture or PDF on a note, or in a ticket's Attachments list, opens it over the ticket.
  • Before 4.4.0, Ctrl-click (or middle-click) a picture to open it in a new tab, which leaves the ticket where it is.

Related

FreeITSM

Getting Started

Modules

Multi-tenancy (planned)

Blue sky thinking

Bugs resolved

Links

Clone this wiki locally