fix: correct popup fallback positioning offsets (#981)

packages/core/src/dom/ui/popover/popover-positioning.ts

@@ -1,4 +1,4 @@
-import { supportsAnchorPositioning } from '@videojs/utils/dom';
+import { resolveCSSLength, supportsAnchorPositioning } from '@videojs/utils/dom';
 import type { PopoverAlign, PopoverSide } from '../../../core/ui/popover/popover-core';
 import { type PopoverCSSVarKey, PopoverCSSVars } from '../../../core/ui/popover/popover-css-vars';
 
@@ -258,7 +258,32 @@ export function getManualPositionStyle(
 export function resolveOffsets(el: Element, cssVars: PositioningCSSVars = PopoverCSSVars): ManualOffsets {
   const computed = getComputedStyle(el);
   return {
-    sideOffset: Number.parseFloat(computed.getPropertyValue(cssVars.sideOffset)) || 0,
-    alignOffset: Number.parseFloat(computed.getPropertyValue(cssVars.alignOffset)) || 0,
+    sideOffset: resolveCSSLength(el, computed.getPropertyValue(cssVars.sideOffset)),
+    alignOffset: resolveCSSLength(el, computed.getPropertyValue(cssVars.alignOffset)),
+  };
+}
+
+/**
+ * Measure the popup's layout box for positioning.
+ *
+ * `getBoundingClientRect()` includes active transforms, which causes the
+ * fallback position to drift while opening/closing animations scale the popup.
+ * Using `offsetWidth`/`offsetHeight` preserves the untransformed size.
+ */
+export function getPopupPositionRect(el: HTMLElement): DOMRect {
+  const rect = el.getBoundingClientRect();
+  const width = el.offsetWidth || rect.width;
+  const height = el.offsetHeight || rect.height;
+  const adjustedRect = {
+    ...rect,
+    width,
+    height,
+    right: rect.left + width,
+    bottom: rect.top + height,
+  };
+
+  return {
+    ...adjustedRect,
+    toJSON: () => adjustedRect,
   };
 }

packages/core/src/dom/ui/popover/popover.ts

@@ -87,6 +87,16 @@ export function createPopover(options: PopoverOptions): PopoverApi {
     return globalThis.matchMedia?.('(hover: hover)')?.matches ?? false;
   }
 
+  function canOpenOnFocus(): boolean {
+    if (!canHover()) return false;
+    return globalThis.matchMedia?.('(pointer: fine)')?.matches ?? false;
+  }
+
+  function canToggleOnClick(): boolean {
+    if (!options.openOnHover?.()) return true;
+    return canHover();
+  }
+
   // --- Open/close ---
 
   /**
@@ -168,6 +178,8 @@ export function createPopover(options: PopoverOptions): PopoverApi {
 
   const triggerProps: PopoverTriggerProps = {
     onClick(event) {
+      if (!canToggleOnClick()) return;
+
       // During a close animation (open=true, status=ending), treat
       // the click as a re-open rather than a second close attempt.
       if (state.current.active && state.current.status !== 'ending') {
@@ -203,6 +215,7 @@ export function createPopover(options: PopoverOptions): PopoverApi {
 
     onFocusIn(_event) {
       if (options.openOnHover?.()) {
+        if (!canOpenOnFocus()) return;
         applyOpen('focus');
       }
     },

packages/core/src/dom/ui/popover/tests/popover-positioning.test.ts

@@ -5,7 +5,9 @@ import {
   getAnchorPositionStyle,
   getManualPositionStyle,
   getPopoverCSSVars,
+  getPopupPositionRect,
   type ManualOffsets,
+  resolveOffsets,
 } from '../popover-positioning';
 
 // Mock supportsAnchorPositioning for deterministic tests.
@@ -182,6 +184,67 @@ describe('getAnchorPositionStyle', () => {
   });
 });
 
+describe('resolveOffsets', () => {
+  it('resolves non-pixel CSS lengths to pixels', () => {
+    const el = document.createElement('div');
+    const getComputedStyleSpy = vi.spyOn(globalThis, 'getComputedStyle').mockImplementation(
+      (target: Element) =>
+        ({
+          fontSize: target === document.documentElement ? '16px' : '14px',
+          getPropertyValue(name: string) {
+            if (name === PopoverCSSVars.sideOffset) return '0.5rem';
+            if (name === PopoverCSSVars.alignOffset) return '1em';
+            return '';
+          },
+        }) as CSSStyleDeclaration
+    );
+
+    expect(resolveOffsets(el)).toEqual({ sideOffset: 8, alignOffset: 14 });
+
+    getComputedStyleSpy.mockRestore();
+  });
+});
+
+describe('getPopupPositionRect', () => {
+  it('uses untransformed layout size when transforms change the client rect', () => {
+    const el = document.createElement('div');
+
+    Object.defineProperty(el, 'offsetWidth', { configurable: true, value: 200 });
+    Object.defineProperty(el, 'offsetHeight', { configurable: true, value: 80 });
+    vi.spyOn(el, 'getBoundingClientRect').mockImplementation(() => makeDOMRect(20, 40, 100, 40));
+
+    const rect = getPopupPositionRect(el);
+
+    expect(rect.left).toBe(20);
+    expect(rect.top).toBe(40);
+    expect(rect.width).toBe(200);
+    expect(rect.height).toBe(80);
+    expect(rect.right).toBe(220);
+    expect(rect.bottom).toBe(120);
+  });
+
+  it('serializes adjusted rect values from toJSON', () => {
+    const el = document.createElement('div');
+
+    Object.defineProperty(el, 'offsetWidth', { configurable: true, value: 200 });
+    Object.defineProperty(el, 'offsetHeight', { configurable: true, value: 80 });
+    vi.spyOn(el, 'getBoundingClientRect').mockImplementation(() => makeDOMRect(20, 40, 100, 40));
+
+    const rect = getPopupPositionRect(el);
+
+    expect(rect.toJSON()).toEqual(
+      expect.objectContaining({
+        left: 20,
+        top: 40,
+        width: 200,
+        height: 80,
+        right: 220,
+        bottom: 120,
+      })
+    );
+  });
+});
+
 // Tests the CSS anchor positioning path via getAnchorPositionStyle with
 // a fresh module import where supportsAnchorPositioning returns true.
 describe('getAnchorPositionStyle (CSS Anchor Positioning)', () => {

packages/core/src/dom/ui/popover/tests/popover.test.ts

@@ -126,6 +126,75 @@ describe('createPopover', () => {
       expect(popover.input.current.status).not.toBe('ending');
       expect(onOpenChange).toHaveBeenCalledWith(true, expect.objectContaining({ reason: 'click' }));
     });
+
+    it('does not open on click on touch devices when openOnHover is enabled', () => {
+      const matchMedia = vi.fn((query: string) => ({
+        matches: query === '(hover: hover)' ? false : false,
+      }));
+      vi.stubGlobal('matchMedia', matchMedia);
+
+      const { popover, onOpenChange } = createTestPopover({
+        openOnHover: () => true,
+      });
+
+      popover.triggerProps.onClick({ preventDefault: vi.fn() } as unknown as UIEvent);
+
+      expect(onOpenChange).not.toHaveBeenCalled();
+      expect(popover.input.current.active).toBe(false);
+
+      vi.unstubAllGlobals();
+    });
+
+    it('does not open via focus on touch devices when openOnHover is enabled', () => {
+      const matchMedia = vi.fn((query: string) => ({
+        matches: query === '(hover: hover)' ? false : false,
+      }));
+      vi.stubGlobal('matchMedia', matchMedia);
+
+      const { popover, onOpenChange } = createTestPopover({
+        openOnHover: () => true,
+      });
+
+      popover.triggerProps.onFocusIn({ relatedTarget: null, preventDefault: vi.fn() });
+
+      expect(onOpenChange).not.toHaveBeenCalled();
+
+      vi.unstubAllGlobals();
+    });
+
+    it('does not open via focus when pointer is not fine', () => {
+      const matchMedia = vi.fn((query: string) => ({
+        matches: query === '(hover: hover)',
+      }));
+      vi.stubGlobal('matchMedia', matchMedia);
+
+      const { popover, onOpenChange } = createTestPopover({
+        openOnHover: () => true,
+      });
+
+      popover.triggerProps.onFocusIn({ relatedTarget: null, preventDefault: vi.fn() });
+
+      expect(onOpenChange).not.toHaveBeenCalled();
+
+      vi.unstubAllGlobals();
+    });
+
+    it('opens via focus when hover and fine pointer are supported', () => {
+      const matchMedia = vi.fn((query: string) => ({
+        matches: query === '(hover: hover)' || query === '(pointer: fine)',
+      }));
+      vi.stubGlobal('matchMedia', matchMedia);
+
+      const { popover, onOpenChange } = createTestPopover({
+        openOnHover: () => true,
+      });
+
+      popover.triggerProps.onFocusIn({ relatedTarget: null, preventDefault: vi.fn() });
+
+      expect(onOpenChange).toHaveBeenCalledWith(true, { reason: 'focus' });
+
+      vi.unstubAllGlobals();
+    });
   });
 
   describe('element setters', () => {