fix(markdown): prevent image links from trapping users

packages/shared/src/components/Markdown.spec.tsx

@@ -0,0 +1,231 @@
+import React from 'react';
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+import { fireEvent, render, screen, within } from '@testing-library/react';
+import Markdown from './Markdown';
+import { LazyModal } from './modals/common/types';
+import { useRequestProtocol } from '../hooks/useRequestProtocol';
+
+const mockOpenModal = jest.fn();
+
+jest.mock('../hooks/useLazyModal', () => ({
+  useLazyModal: () => ({ openModal: mockOpenModal }),
+}));
+
+jest.mock('../hooks/useDomPurify', () => ({
+  useDomPurify: () => ({ sanitize: (content: string) => content }),
+}));
+
+jest.mock('../hooks/useRequestProtocol', () => ({
+  useRequestProtocol: jest.fn(),
+}));
+
+const mockUseRequestProtocol = useRequestProtocol as jest.MockedFunction<
+  typeof useRequestProtocol
+>;
+
+const renderMarkdown = (content: string, container?: HTMLElement) => {
+  const client = new QueryClient();
+
+  return render(
+    <QueryClientProvider client={client}>
+      <Markdown content={content} />
+    </QueryClientProvider>,
+    { container },
+  );
+};
+
+describe('Markdown image interactions', () => {
+  const imageUrl = 'https://media.daily.dev/image/upload/f_auto/v1/posts/abc';
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    mockUseRequestProtocol.mockReturnValue({
+      requestMethod: jest.fn(),
+      fetchMethod: jest.fn(),
+      isCompanion: false,
+    } as unknown as ReturnType<typeof useRequestProtocol>);
+  });
+
+  it('opens a linked image in the lightbox and prevents navigation', () => {
+    renderMarkdown(
+      `<a href="${imageUrl}" target="_blank" rel="noopener nofollow ugc"><img src="${imageUrl}" alt="Screenshot" /></a>`,
+    );
+
+    const image = screen.getByRole('button', { name: 'Open image' });
+    const event = new MouseEvent('click', {
+      bubbles: true,
+      cancelable: true,
+    });
+
+    image.dispatchEvent(event);
+
+    expect(event.defaultPrevented).toBe(true);
+    expect(mockOpenModal).toHaveBeenCalledTimes(1);
+    expect(mockOpenModal).toHaveBeenCalledWith(
+      expect.objectContaining({
+        type: LazyModal.ImageView,
+        props: expect.objectContaining({
+          src: imageUrl,
+          alt: 'Screenshot',
+          originRect: expect.objectContaining({
+            top: expect.any(Number),
+            left: expect.any(Number),
+            width: expect.any(Number),
+            height: expect.any(Number),
+          }),
+        }),
+      }),
+    );
+  });
+
+  it('opens a linked image from the keyboard and prevents navigation', () => {
+    renderMarkdown(
+      `<a href="${imageUrl}" target="_blank" rel="noopener nofollow ugc"><img src="${imageUrl}" alt="Screenshot" /></a>`,
+    );
+
+    const image = screen.getByRole('button', { name: 'Open image' });
+
+    expect(fireEvent.keyDown(image, { key: 'Enter' })).toBe(false);
+    expect(mockOpenModal).toHaveBeenCalledTimes(1);
+    expect(mockOpenModal).toHaveBeenCalledWith(
+      expect.objectContaining({
+        type: LazyModal.ImageView,
+        props: expect.objectContaining({
+          src: imageUrl,
+          alt: 'Screenshot',
+        }),
+      }),
+    );
+  });
+
+  it('allows an image wrapped by a non-image link to follow that link once', () => {
+    const linkUrl = 'https://github.com/dailydotdev/apps';
+    renderMarkdown(
+      `<a href="${linkUrl}" target="_blank" rel="noopener nofollow ugc"><img src="${imageUrl}" alt="Repository badge" /></a>`,
+    );
+
+    const image = screen.getByRole('button', { name: 'Open image' });
+    const anchor = image.closest('a');
+    const onAnchorClick = jest.fn();
+
+    anchor?.addEventListener('click', onAnchorClick);
+    fireEvent.click(image);
+
+    expect(onAnchorClick).toHaveBeenCalledTimes(1);
+    expect(mockOpenModal).not.toHaveBeenCalled();
+  });
+
+  it('allows an image wrapped by a non-image link to follow that link from the keyboard', () => {
+    const linkUrl = 'https://github.com/dailydotdev/apps';
+    renderMarkdown(
+      `<a href="${linkUrl}" target="_blank" rel="noopener nofollow ugc"><img src="${imageUrl}" alt="Repository badge" /></a>`,
+    );
+
+    const image = screen.getByRole('button', { name: 'Open image' });
+    const anchor = image.closest('a');
+    const onAnchorClick = jest.fn();
+
+    anchor?.addEventListener('click', onAnchorClick);
+    expect(fireEvent.keyDown(image, { key: 'Enter' })).toBe(false);
+
+    expect(onAnchorClick).toHaveBeenCalledTimes(1);
+    expect(mockOpenModal).not.toHaveBeenCalled();
+  });
+
+  it('opens a bare image in the lightbox', () => {
+    renderMarkdown(`<img src="${imageUrl}" alt="Screenshot" />`);
+
+    fireEvent.click(screen.getByRole('button', { name: 'Open image' }));
+
+    expect(mockOpenModal).toHaveBeenCalledTimes(1);
+    expect(mockOpenModal).toHaveBeenCalledWith(
+      expect.objectContaining({
+        type: LazyModal.ImageView,
+        props: expect.objectContaining({
+          src: imageUrl,
+          alt: 'Screenshot',
+        }),
+      }),
+    );
+  });
+
+  it('opens a text link to an image in the lightbox', () => {
+    renderMarkdown(
+      `<a href="https://example.com/screenshot.png?raw=1" target="_blank" rel="noopener nofollow ugc">screenshot</a>`,
+    );
+
+    fireEvent.click(screen.getByRole('link', { name: 'screenshot' }));
+
+    expect(mockOpenModal).toHaveBeenCalledTimes(1);
+    expect(mockOpenModal).toHaveBeenCalledWith(
+      expect.objectContaining({
+        type: LazyModal.ImageView,
+        props: expect.objectContaining({
+          src: 'https://example.com/screenshot.png?raw=1',
+          alt: 'screenshot',
+        }),
+      }),
+    );
+  });
+
+  it('allows text links to daily media videos to navigate', () => {
+    const { container } = renderMarkdown(
+      '<a href="https://media.daily.dev/video/upload/v1/posts/clip.mp4" target="_blank">Watch video</a>',
+      document.createElement('div'),
+    );
+    const { getByRole } = within(container);
+
+    expect(fireEvent.click(getByRole('link', { name: 'Watch video' }))).toBe(
+      true,
+    );
+    expect(mockOpenModal).not.toHaveBeenCalled();
+  });
+
+  describe.each(['image', 'text'])('%s links', (contentType) => {
+    it.each([
+      { metaKey: true },
+      { ctrlKey: true },
+      { shiftKey: true },
+      { altKey: true },
+      { button: 1 },
+    ])('preserves browser navigation for clicks with %j', (options) => {
+      const content =
+        contentType === 'image'
+          ? `<img src="${imageUrl}" alt="Screenshot" />`
+          : 'Screenshot';
+      const { container } = renderMarkdown(
+        `<a href="${imageUrl}" target="_blank">${content}</a>`,
+        document.createElement('div'),
+      );
+      const { getByRole } = within(container);
+
+      const target =
+        contentType === 'image'
+          ? getByRole('button', { name: 'Open image' })
+          : getByRole('link', { name: 'Screenshot' });
+
+      expect(fireEvent.click(target, options)).toBe(true);
+      expect(mockOpenModal).not.toHaveBeenCalled();
+    });
+  });
+
+  it('opens image links in a new tab for the companion', () => {
+    mockUseRequestProtocol.mockReturnValue({
+      requestMethod: jest.fn(),
+      fetchMethod: jest.fn(),
+      isCompanion: true,
+    } as unknown as ReturnType<typeof useRequestProtocol>);
+
+    renderMarkdown(`<img src="${imageUrl}" alt="Screenshot" />`);
+
+    fireEvent.click(screen.getByRole('button', { name: 'Open image' }));
+
+    expect(window.open).toHaveBeenCalledTimes(1);
+    expect(window.open).toHaveBeenCalledWith(
+      imageUrl,
+      '_blank',
+      'noopener,noreferrer',
+    );
+    expect(mockOpenModal).not.toHaveBeenCalled();
+  });
+});

packages/shared/src/components/Markdown.tsx

@@ -18,13 +18,62 @@ import { useLazyModal } from '../hooks/useLazyModal';
 import { LazyModal } from './modals/common/types';
 import { getImageOriginRect } from './modals/ImageModal';
 import { useRequestProtocol } from '../hooks/useRequestProtocol';
+import { isImageUrl } from '../lib/image';
 
 function isImageElement(
   element: Element | EventTarget,
 ): element is HTMLImageElement {
   return element instanceof HTMLImageElement;
 }
 
+function getTargetElement(target: EventTarget): Element | null {
+  if (target instanceof Element) {
+    return target;
+  }
+
+  if (target instanceof Node) {
+    return target.parentElement;
+  }
+
+  return null;
+}
+
+function getWrappingAnchor(
+  element: Element,
+  container: HTMLElement | null,
+): HTMLAnchorElement | null {
+  let currentElement: Element | null = element;
+
+  while (currentElement && currentElement !== container) {
+    if (currentElement instanceof HTMLAnchorElement) {
+      return currentElement;
+    }
+
+    currentElement = currentElement.parentElement;
+  }
+
+  return null;
+}
+
+function isSameUrl(url: string, otherUrl: string): boolean {
+  try {
+    return new URL(url).href === new URL(otherUrl).href;
+  } catch {
+    return url === otherUrl;
+  }
+}
+
+function shouldOpenAnchorImage(
+  anchor: HTMLAnchorElement | null,
+  imageSrc: string,
+): boolean {
+  if (!anchor?.href) {
+    return true;
+  }
+
+  return isImageUrl(anchor.href) || isSameUrl(anchor.href, imageSrc);
+}
+
 const UserEntityCard = dynamic(() => import('./cards/entity/UserEntityCard'), {
   ssr: false,
 });
@@ -120,19 +169,19 @@ export default function Markdown({
   );
 
   const openImage = useCallback(
-    (element: HTMLImageElement) => {
+    (src: string, alt: string | undefined, originElement: Element) => {
       // The lazy-modal renderer isn't mounted in the extension companion, so
       // fall back to opening the image in a new tab there.
       if (isCompanion) {
-        window.open(element.src, '_blank', 'noopener,noreferrer');
+        window.open(src, '_blank', 'noopener,noreferrer');
         return;
       }
       openModal({
         type: LazyModal.ImageView,
         props: {
-          src: element.src,
-          alt: element.alt || undefined,
-          originRect: getImageOriginRect(element),
+          src,
+          alt,
+          originRect: getImageOriginRect(originElement),
         },
       });
     },
@@ -141,28 +190,69 @@ export default function Markdown({
 
   const onImageClick = useCallback(
     (e: MouseEvent<HTMLDivElement>) => {
-      const element = e.target;
+      if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey || e.button !== 0) {
+        return;
+      }
+
+      const element = getTargetElement(e.target);
+
+      if (!element) {
+        return;
+      }
+
+      const anchor = getWrappingAnchor(element, containerRef.current);
 
       if (isImageElement(element) && element.src) {
         e.stopPropagation();
-        openImage(element);
+
+        if (!shouldOpenAnchorImage(anchor, element.src)) {
+          return;
+        }
+
+        e.preventDefault();
+        openImage(
+          anchor?.href || element.src,
+          element.alt || undefined,
+          element,
+        );
+        return;
+      }
+
+      if (anchor?.href && isImageUrl(anchor.href)) {
+        e.preventDefault();
+        e.stopPropagation();
+        openImage(anchor.href, anchor.textContent?.trim() || undefined, anchor);
       }
     },
     [openImage],
   );
 
   const onImageKeyDown = useCallback(
     (e: KeyboardEvent<HTMLDivElement>) => {
-      const element = e.target;
+      const element = getTargetElement(e.target);
 
       if (
+        element &&
         isImageElement(element) &&
         element.src &&
         (e.key === 'Enter' || e.key === ' ')
       ) {
-        e.preventDefault();
+        const anchor = getWrappingAnchor(element, containerRef.current);
+
         e.stopPropagation();
-        openImage(element);
+
+        if (!shouldOpenAnchorImage(anchor, element.src)) {
+          e.preventDefault();
+          anchor?.click();
+          return;
+        }
+
+        e.preventDefault();
+        openImage(
+          anchor?.href || element.src,
+          element.alt || undefined,
+          element,
+        );
       }
     },
     [openImage],