From 0f4c9b82121f870e5c925657458a9b87912f59ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Fri, 7 Aug 2026 09:36:52 -0400
Subject: [PATCH] fix(ui): don't start playback when closing the disc cover
 lightbox (#5901)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The Lightbox renders through a React portal (react-modal), and React
propagates synthetic events up the React tree rather than the DOM tree, so
clicking its close button or backdrop bubbled into the ancestor TableRow and
triggered playSubset.

The existing timestamp guard could not catch this: react-image-lightbox defers
onCloseRequest by animationDuration via setTimeout, so lightboxClosedAt was
only stamped 200ms after the click had already propagated. The guard is kept
because it still covers the separate mobile ghost-click path, where the
synthesized click lands on the row after the overlay unmounts.

Stopping propagation in onCloseRequest — the approach used by ContextMenus,
whose MUI onClose fires synchronously — does not work for the same reason.
Instead, wrap the Lightbox in an element that stops propagation at the React
tree boundary the portal bubbles through, which also covers the lightbox
controls we don't own (zoom buttons, caption, image drag).
---
 ui/src/common/SongDatagrid.jsx      | 20 ++++----
 ui/src/common/SongDatagrid.test.jsx | 74 +++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 8 deletions(-)
 create mode 100644 ui/src/common/SongDatagrid.test.jsx

diff --git a/ui/src/common/SongDatagrid.jsx b/ui/src/common/SongDatagrid.jsx
index d2c98bbe7..4a813f84d 100644
--- a/ui/src/common/SongDatagrid.jsx
+++ b/ui/src/common/SongDatagrid.jsx
@@ -78,7 +78,7 @@ const useStyles = makeStyles({
   },
 })
 
-const DiscSubtitleRow = forwardRef(
+export const DiscSubtitleRow = forwardRef(
   ({ record, onClick, colSpan, contextAlwaysVisible }, ref) => {
     const translate = useTranslate()
     const isDesktop = useMediaQuery((theme) => theme.breakpoints.up('md'))
@@ -153,13 +153,17 @@ const DiscSubtitleRow = forwardRef(
             {subtitle}
           </Typography>
           {isLightboxOpen && !imageError && (
-            <Lightbox
-              imagePadding={50}
-              animationDuration={200}
-              imageTitle={record.album + ' - ' + subtitle}
-              mainSrc={fullImageUrl}
-              onCloseRequest={handleCloseLightbox}
-            />
+            // Lightbox portals out of the row, but React still bubbles its
+            // events up this tree, where the row's onClick would play the disc.
+            <span onClick={(e) => e.stopPropagation()}>
+              <Lightbox
+                imagePadding={50}
+                animationDuration={200}
+                imageTitle={record.album + ' - ' + subtitle}
+                mainSrc={fullImageUrl}
+                onCloseRequest={handleCloseLightbox}
+              />
+            </span>
           )}
         </TableCell>
         <TableCell>
diff --git a/ui/src/common/SongDatagrid.test.jsx b/ui/src/common/SongDatagrid.test.jsx
new file mode 100644
index 000000000..f8459d8eb
--- /dev/null
+++ b/ui/src/common/SongDatagrid.test.jsx
@@ -0,0 +1,74 @@
+import React from 'react'
+import { render, fireEvent, screen } from '@testing-library/react'
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { createTheme, ThemeProvider } from '@material-ui/core/styles'
+import { DiscSubtitleRow } from './SongDatagrid'
+
+vi.mock('../subsonic', () => ({
+  default: { getDiscCoverArtUrl: () => 'http://localhost/cover.jpg' },
+}))
+
+vi.mock('react-redux', () => ({ useDispatch: () => vi.fn() }))
+
+vi.mock('../common', () => ({ AlbumContextMenu: () => null }))
+
+vi.mock('react-dnd', () => ({ useDrag: () => [{}, vi.fn()] }))
+
+const record = {
+  id: 'song-1',
+  albumId: 'album-1',
+  album: 'The Album',
+  discNumber: 2,
+  discSubtitle: 'Bonus Disc',
+  updatedAt: '2024-01-01',
+}
+
+const renderRow = (onClick) =>
+  render(
+    <ThemeProvider theme={createTheme()}>
+      <table>
+        <tbody>
+          <DiscSubtitleRow record={record} onClick={onClick} colSpan={3} />
+        </tbody>
+      </table>
+    </ThemeProvider>,
+  )
+
+const openLightbox = () => {
+  fireEvent.click(document.querySelector('img'))
+  expect(document.querySelector('.ril__closeButton')).toBeTruthy()
+}
+
+describe('DiscSubtitleRow', () => {
+  beforeEach(() => vi.clearAllMocks())
+
+  it('plays the disc when the row is clicked', () => {
+    const onClick = vi.fn()
+    renderRow(onClick)
+    fireEvent.click(screen.getByText('Bonus Disc'))
+    expect(onClick).toHaveBeenCalledWith(2)
+  })
+
+  it('does not play the disc when opening the lightbox', () => {
+    const onClick = vi.fn()
+    renderRow(onClick)
+    openLightbox()
+    expect(onClick).not.toHaveBeenCalled()
+  })
+
+  it('does not play the disc when closing the lightbox', () => {
+    const onClick = vi.fn()
+    renderRow(onClick)
+    openLightbox()
+    fireEvent.click(document.querySelector('.ril__closeButton'))
+    expect(onClick).not.toHaveBeenCalled()
+  })
+
+  it('does not play the disc when clicking the lightbox backdrop', () => {
+    const onClick = vi.fn()
+    renderRow(onClick)
+    openLightbox()
+    fireEvent.click(document.querySelector('.ril__inner'))
+    expect(onClick).not.toHaveBeenCalled()
+  })
+})