From 351e8e23593c3afcf8d2dbb635c176aa612a5dec Mon Sep 17 00:00:00 2001
From: David Battersby <info@davidbattersby.com>
Date: Fri, 21 Jul 2023 12:28:37 +0800
Subject: [PATCH] FIX: capture click target in lightbox click handler (#22732)

In Safari, clicking any image in a lightbox gallery results in the first image loading (instead of the clicked image).

Previously we relied on document.activeElement to determine which lightbox image was clicked. However in Chrome the active element is the lightbox selector (a.lightbox), whereas in Safari the active element defaults to the body tag.

Currently the startingIndex that is calculated within processHTML() is used by lightbox to determine which image to load first. The starting index is currently achieved by checking each lightbox element within the gallery against the active element.

To fix this issue we can use the event.target to get the clicked image, then use the closest selector and pass that into the function to do the matching and return the correct startingIndex.
---
 .../discourse/app/lib/lightbox/process-html.js         |  6 ++----
 .../javascripts/discourse/app/services/lightbox.js     | 10 ++++++++--
 .../discourse/tests/unit/services/lightbox-test.js     |  5 +++--
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/lib/lightbox/process-html.js b/app/assets/javascripts/discourse/app/lib/lightbox/process-html.js
index b8bd24bd889..cf65e807383 100644
--- a/app/assets/javascripts/discourse/app/lib/lightbox/process-html.js
+++ b/app/assets/javascripts/discourse/app/lib/lightbox/process-html.js
@@ -2,14 +2,12 @@ import { SELECTORS } from "./constants";
 import { escapeExpression } from "discourse/lib/utilities";
 import { htmlSafe } from "@ember/template";
 
-export async function processHTML({ container, selector }) {
+export async function processHTML({ container, selector, clickTarget }) {
   selector ??= SELECTORS.DEFAULT_ITEM_SELECTOR;
 
   const items = [...container.querySelectorAll(selector)];
 
-  let _startingIndex = items.findIndex(
-    (item) => item === document.activeElement
-  );
+  let _startingIndex = items.findIndex((item) => item === clickTarget);
 
   if (_startingIndex === -1) {
     _startingIndex = 0;
diff --git a/app/assets/javascripts/discourse/app/services/lightbox.js b/app/assets/javascripts/discourse/app/services/lightbox.js
index ee7aa73728c..884d8d8154b 100644
--- a/app/assets/javascripts/discourse/app/services/lightbox.js
+++ b/app/assets/javascripts/discourse/app/services/lightbox.js
@@ -107,8 +107,12 @@ export default class LightboxService extends Service {
   }
 
   @bind
-  async openLightbox({ container, selector }) {
-    const { items, startingIndex } = await processHTML({ container, selector });
+  async openLightbox({ container, selector, clickTarget }) {
+    const { items, startingIndex } = await processHTML({
+      container,
+      selector,
+      clickTarget,
+    });
 
     if (!items.length) {
       return;
@@ -236,6 +240,7 @@ export default class LightboxService extends Service {
   }
 
   handleClickEvent(event, trigger) {
+    const closestTrigger = event.target.closest(trigger);
     const isLightboxClick = event
       .composedPath()
       .find(
@@ -254,6 +259,7 @@ export default class LightboxService extends Service {
     this.openLightbox({
       container: event.currentTarget,
       selector: trigger,
+      clickTarget: closestTrigger,
     });
 
     event.target.toggleAttribute(SELECTORS.DOCUMENT_LAST_FOCUSED_ELEMENT);
diff --git a/app/assets/javascripts/discourse/tests/unit/services/lightbox-test.js b/app/assets/javascripts/discourse/tests/unit/services/lightbox-test.js
index b96831b5dda..be359d19c64 100644
--- a/app/assets/javascripts/discourse/tests/unit/services/lightbox-test.js
+++ b/app/assets/javascripts/discourse/tests/unit/services/lightbox-test.js
@@ -60,8 +60,9 @@ module("Unit | Service | Experimental Lightbox", function (hooks) {
 
     const openLightboxSpy = sinon.spy(this.lightbox, "openLightbox");
     const removeEventListenerSpy = sinon.spy(container, "removeEventListener");
+    const clickTarget = container.querySelector(selector);
 
-    await this.lightbox.setupLightboxes({ container, selector });
+    await this.lightbox.setupLightboxes({ container, selector, clickTarget });
 
     await click(container.querySelector(selector));
 
@@ -70,7 +71,7 @@ module("Unit | Service | Experimental Lightbox", function (hooks) {
     await click(container.querySelector("p"));
 
     assert.strictEqual(
-      openLightboxSpy.calledWith({ container, selector }),
+      openLightboxSpy.calledWith({ container, selector, clickTarget }),
       true,
       "calls openLightbox on lightboxed element click"
     );