Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 61 additions & 7 deletions src/Popup/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ import PopupContent from './PopupContent';
import useOffsetStyle from '../hooks/useOffsetStyle';
import { useEvent } from '@rc-component/util';
import type { PortalProps } from '@rc-component/portal';
import {
focusPopupRootOrFirst,
handlePopupTabTrap,
} from '../focusUtils';

export interface MobileConfig {
mask?: boolean;
Expand Down Expand Up @@ -85,6 +89,12 @@ export interface PopupProps {

// Mobile
mobile?: MobileConfig;

/**
* Move focus into the popup when it opens and return it to `target` when it closes.
* Tab cycles within the popup. Escape is handled by Portal `onEsc`.
*/
focusPopup?: boolean;
}

const Popup = React.forwardRef<HTMLDivElement, PopupProps>((props, ref) => {
Expand Down Expand Up @@ -149,8 +159,13 @@ const Popup = React.forwardRef<HTMLDivElement, PopupProps>((props, ref) => {
stretch,
targetWidth,
targetHeight,

focusPopup,
} = props;

const rootRef = React.useRef<HTMLDivElement>(null);
const prevOpenRef = React.useRef(false);

const popupContent = typeof popup === 'function' ? popup() : popup;

// We can not remove holder only when motion finished.
Expand Down Expand Up @@ -208,12 +223,7 @@ const Popup = React.forwardRef<HTMLDivElement, PopupProps>((props, ref) => {
offsetY,
);

// ========================= Render =========================
if (!show) {
return null;
}

// >>>>> Misc
// >>>>> Misc (computed before conditional return; hooks must run every render)
const miscStyle: React.CSSProperties = {};
if (stretch) {
if (stretch.includes('height') && targetHeight) {
Expand All @@ -232,6 +242,49 @@ const Popup = React.forwardRef<HTMLDivElement, PopupProps>((props, ref) => {
miscStyle.pointerEvents = 'none';
}

useLayoutEffect(() => {
if (!focusPopup) {
prevOpenRef.current = open;
return;
}

const root = rootRef.current;
const wasOpen = prevOpenRef.current;
prevOpenRef.current = open;

if (open && !wasOpen && root && isNodeVisible) {
focusPopupRootOrFirst(root);
} else if (!open && wasOpen && root) {
const active = document.activeElement as HTMLElement | null;
if (
target &&
active &&
(root === active || root.contains(active))
) {
if (target.isConnected) {
target.focus();
}
Comment on lines +255 to +266
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 只读核验:定位当前实现中 activeElement 的 document 作用域
rg -n "activeElement" src/Popup/index.tsx src/focusUtils.ts

Repository: react-component/trigger

Length of output: 241


🏁 Script executed:

# 查看src/Popup/index.tsx第255-270行的完整上下文
sed -n '255,270p' src/Popup/index.tsx

Repository: react-component/trigger

Length of output: 506


🏁 Script executed:

# 查看src/focusUtils.ts中activeElement的完整上下文
sed -n '60,75p' src/focusUtils.ts

Repository: react-component/trigger

Length of output: 407


🏁 Script executed:

# 检查root对象的声明和初始化,确认它是否总是可用
sed -n '1,260p' src/Popup/index.tsx | grep -n "root" | tail -20

Repository: react-component/trigger

Length of output: 305


🏁 Script executed:

# 搜索与iframe或跨document相关的测试或文档
rg -i "iframe|cross.document|ownerDocument" --type ts --type tsx

Repository: react-component/trigger

Length of output: 94


🏁 Script executed:

# 检查是否有其他地方也使用了document.activeElement且未使用ownerDocument
rg -n "document\.activeElement" --type ts --type tsx

Repository: react-component/trigger

Length of output: 94


🏁 Script executed:

# 查看focusUtils.ts的完整容器相关上下文
sed -n '50,85p' src/focusUtils.ts

Repository: react-component/trigger

Length of output: 834


🏁 Script executed:

# 查看src/Popup/index.tsx更多上下文,理解root的生命周期
sed -n '240,275p' src/Popup/index.tsx

Repository: react-component/trigger

Length of output: 927


🏁 Script executed:

# 搜索ownerDocument的现有使用
rg "ownerDocument" --type ts --type javascript

Repository: react-component/trigger

Length of output: 101


🏁 Script executed:

# 搜索与焦点相关的测试文件
fd -e "test" -e "spec"

Repository: react-component/trigger

Length of output: 49


🏁 Script executed:

# 检查是否有iframe相关的使用场景
rg -i "iframe" .

Repository: react-component/trigger

Length of output: 49


🏁 Script executed:

# 验证HTMLElement是否有ownerDocument属性(应该有)
rg "ownerDocument" --type ts --type tsx -B 3 -A 3

Repository: react-component/trigger

Length of output: 94


🏁 Script executed:

# 查看focusUtils.ts中handlePopupTabTrap的完整函数签名和使用
rg -B 5 -A 15 "function handlePopupTabTrap"

Repository: react-component/trigger

Length of output: 857


🏁 Script executed:

# 检查root对象的类型定义
sed -n '166,170p' src/Popup/index.tsx

Repository: react-component/trigger

Length of output: 235


🏁 Script executed:

# 搜索where handlePopupTabTrap被调用
rg "handlePopupTabTrap" --type ts --type tsx

Repository: react-component/trigger

Length of output: 94


🏁 Script executed:

# 检查是否有任何关于focus管理或文档隔离的注释或文档
rg -i "focus|document" README.md CHANGELOG.md 2>/dev/null | head -30

Repository: react-component/trigger

Length of output: 385


🏁 Script executed:

# 确认focusUtils.ts的同一问题,获取完整上下文
sed -n '55,75p' src/focusUtils.ts

Repository: react-component/trigger

Length of output: 498


🏁 Script executed:

# 验证root.contains是否跨document兼容
rg -B 5 -A 5 "root.contains"

Repository: react-component/trigger

Length of output: 582


跨 document 场景下回焦判定失效

Line 258 使用全局 document.activeElement,在 iframe、Shadow DOM 等跨 document 容器中,该元素属于主文档而非 popup 所在文档,导致 root.contains(active) 判定失败,回焦功能无法生效。

建议改为 root.ownerDocument.activeElement,同一问题也存在于 src/focusUtils.ts:66handlePopupTabTrap 函数中。

🔧 修复建议

src/Popup/index.tsx

-      const active = document.activeElement as HTMLElement | null;
+      const active = root.ownerDocument.activeElement as HTMLElement | null;

src/focusUtils.ts

-  const active = document.activeElement as HTMLElement | null;
+  const active = container.ownerDocument.activeElement as HTMLElement | null;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Popup/index.tsx` around lines 255 - 266, The blur/refocus check uses the
global document.activeElement which breaks across documents (iframes/Shadow
DOM); update the logic in Popup's close branch (the block using root, target and
calling target.focus()—function/variables: focusPopupRootOrFirst, root, target)
to read active via root.ownerDocument.activeElement instead of
document.activeElement; apply the same change inside src/focusUtils.ts in
handlePopupTabTrap (replace document.activeElement with
root.ownerDocument.activeElement or the appropriate root/doc reference used
there) so cross-document focus containment checks (root.contains(active)) work
correctly.

}
}
}, [open, focusPopup, isNodeVisible, target]);

const onPopupKeyDownCapture = useEvent(
(e: React.KeyboardEvent<HTMLDivElement>) => {
if (!focusPopup || !open) {
return;
}
const root = rootRef.current;
if (root) {
handlePopupTabTrap(e, root);
}
},
);

// ========================= Render =========================
if (!show) {
return null;
}

return (
<Portal
open={forceRender || isNodeVisible}
Expand Down Expand Up @@ -276,7 +329,7 @@ const Popup = React.forwardRef<HTMLDivElement, PopupProps>((props, ref) => {

return (
<div
ref={composeRef(resizeObserverRef, ref, motionRef)}
ref={composeRef(resizeObserverRef, ref, motionRef, rootRef)}
className={cls}
style={
{
Expand All @@ -295,6 +348,7 @@ const Popup = React.forwardRef<HTMLDivElement, PopupProps>((props, ref) => {
onPointerEnter={onPointerEnter}
onClick={onClick}
onPointerDownCapture={onPointerDownCapture}
onKeyDownCapture={onPopupKeyDownCapture}
>
{arrow && (
<Arrow
Expand Down
1 change: 1 addition & 0 deletions src/UniqueProvider/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ const UniqueProvider = ({
motion={mergedOptions.popupMotion}
maskMotion={mergedOptions.maskMotion}
getPopupContainer={mergedOptions.getPopupContainer}
focusPopup={mergedOptions.focusPopup}
>
<UniqueContainer
prefixCls={prefixCls}
Expand Down
1 change: 1 addition & 0 deletions src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface UniqueShowOptions {
getPopupContainer?: TriggerProps['getPopupContainer'];
getPopupClassNameFromAlign?: (align: AlignType) => string;
onEsc?: PortalProps['onEsc'];
focusPopup?: boolean;
}

export interface UniqueContextProps {
Expand Down
91 changes: 91 additions & 0 deletions src/focusUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import type * as React from 'react';

const TABBABLE_SELECTOR =
'a[href], button, input, select, textarea, [tabindex]:not([tabindex="-1"])';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The TABBABLE_SELECTOR is missing some naturally tabbable elements such as summary, [contenteditable], and media elements with controls (audio[controls], video[controls]). Consider expanding the selector for better accessibility coverage.

const TABBABLE_SELECTOR =
  'a[href], button, input, select, textarea, summary, [contenteditable], audio[controls], video[controls], [tabindex]:not([tabindex="-1"])';


function isTabbable(el: HTMLElement, win: Window): boolean {
if (el.closest('[aria-hidden="true"]')) {
return false;
}
if ('disabled' in el && (el as HTMLButtonElement).disabled) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Checking for the disabled property on the element itself does not account for elements disabled via a parent <fieldset disabled>. Using el.matches(':disabled') is a more robust way to check if an element is effectively disabled in the DOM.

Suggested change
if ('disabled' in el && (el as HTMLButtonElement).disabled) {
if (el.matches(':disabled')) {
return false;
}

return false;
}
if (el instanceof HTMLInputElement && el.type === 'hidden') {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 只读核验:定位跨 realm 敏感写法
rg -n "instanceof HTMLInputElement|document\.activeElement" src/focusUtils.ts src/Popup/index.tsx

Repository: react-component/trigger

Length of output: 326


🏁 Script executed:

cat -n src/focusUtils.ts | head -80

Repository: react-component/trigger

Length of output: 2681


🏁 Script executed:

cat -n src/Popup/index.tsx | sed -n '250,270p'

Repository: react-component/trigger

Length of output: 762


🏁 Script executed:

# Check if there are any tests related to focusUtils
fd -e test -e spec | xargs grep -l "focusUtils\|isTabbable\|handlePopupTabTrap" 2>/dev/null | head -5

Repository: react-component/trigger

Length of output: 49


🏁 Script executed:

# Check for any tests or documentation about iframe/cross-realm support
fd -type f \( -name "*.test.*" -o -name "*.spec.*" -o -name "README*" \) | xargs grep -l "iframe\|cross.realm\|cross.document" 2>/dev/null

Repository: react-component/trigger

Length of output: 239


🏁 Script executed:

# Check git history or issues directory for cross-realm discussions
find . -name "*.md" -o -name "*.txt" | xargs grep -i "iframe\|cross.realm\|cross.document\|cross.window" 2>/dev/null | head -20

Repository: react-component/trigger

Length of output: 49


🏁 Script executed:

# Check if there are existing focus-related tests
fd -type f \( -name "*.test.*" -o -name "*.spec.*" \) | xargs grep -l "focus\|tabbable\|focusUtils" 2>/dev/null

Repository: react-component/trigger

Length of output: 239


🏁 Script executed:

# Find test files with correct syntax
fd "\.test\.|\.spec\." | head -20

Repository: react-component/trigger

Length of output: 465


🏁 Script executed:

# Search for focus-related tests
rg -l "focus|tabbable|focusUtils" --type ts --type tsx --type js --type jsx 2>/dev/null | grep -i "test\|spec"

Repository: react-component/trigger

Length of output: 49


🏁 Script executed:

# Check if container.contains works cross-document
rg -n "contains" src/focusUtils.ts

Repository: react-component/trigger

Length of output: 117


🏁 Script executed:

# Look for any cross-document or shadow DOM concerns in the codebase
rg -i "ownerDocument|defaultView|cross.document|shadow" src/ | head -20

Repository: react-component/trigger

Length of output: 1286


🏁 Script executed:

cat tests/focus.test.tsx

Repository: react-component/trigger

Length of output: 5444


🏁 Script executed:

# Check what win parameter is used for in isTabbable
rg -A 5 "function isTabbable" src/focusUtils.ts

Repository: react-component/trigger

Length of output: 275


跨 realm(iframe)下的 instanceof 判定与 activeElement 读取需修正。

Line 13 使用全局 HTMLInputElement 进行 instanceof 判定,但 isTabbable 已接收 win 参数(来自 container.ownerDocument.defaultView);当容器在不同 document 中时判定失效。
Line 66 使用全局 document.activeElement 而应使用 container.ownerDocument.activeElement,虽然 container.contains() 提供了部分保护但不是完整解决方案。

建议修正:

修复方案
 function isTabbable(el: HTMLElement, win: Window): boolean {
-  if (el instanceof HTMLInputElement && el.type === 'hidden') {
+  if (el instanceof win.HTMLInputElement && el.type === 'hidden') {
     return false;
   }

 export function handlePopupTabTrap(
   e: React.KeyboardEvent,
   container: HTMLElement,
 ): void {
-  const active = document.activeElement as HTMLElement | null;
+  const active = container.ownerDocument.activeElement as HTMLElement | null;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (el instanceof HTMLInputElement && el.type === 'hidden') {
function isTabbable(el: HTMLElement, win: Window): boolean {
if (el instanceof win.HTMLInputElement && el.type === 'hidden') {
return false;
}
// ... rest of function
}
export function handlePopupTabTrap(
e: React.KeyboardEvent,
container: HTMLElement,
): void {
const active = container.ownerDocument.activeElement as HTMLElement | null;
// ... rest of function
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/focusUtils.ts` at line 13, The instanceof check in isTabbable should use
the frame's globals rather than the top-level ones: change references from the
global HTMLInputElement to win.HTMLInputElement (where win is the defaultView
passed into isTabbable) so cross-realm inputs are detected correctly, and
replace uses of document.activeElement with
container.ownerDocument.activeElement (or win.document.activeElement) so
activeElement is read from the same document as the container; update the checks
in the isTabbable function and any other focus-related helpers that use global
DOM constructors or document.activeElement to use the passed-in
win/container.ownerDocument to ensure correct behavior across iframes.

return false;
}
const ti = el.getAttribute('tabindex');
if (ti !== null && Number(ti) < 0) {
return false;
}
const style = win.getComputedStyle(el);
if (style.display === 'none' || style.visibility === 'hidden') {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Checking display and visibility on the element itself via getComputedStyle does not detect if the element is hidden because one of its ancestors has display: none. A more reliable check for layout visibility is el.getClientRects().length > 0 or checking el.offsetParent === null (though the latter has edge cases for fixed elements).

Suggested change
if (style.display === 'none' || style.visibility === 'hidden') {
if (el.getClientRects().length === 0) {
return false;
}

return false;
}
return true;
}

/** Visible, tabbable descendants inside `container` (in DOM order). */
export function getTabbableElements(container: HTMLElement): HTMLElement[] {
const doc = container.ownerDocument;
const win = doc.defaultView!;
const nodeList = container.querySelectorAll<HTMLElement>(TABBABLE_SELECTOR);
const list: HTMLElement[] = [];
for (let i = 0; i < nodeList.length; i += 1) {
const el = nodeList[i];
if (isTabbable(el, win)) {
list.push(el);
}
}
return list;
}

export function focusPopupRootOrFirst(
container: HTMLElement,
): HTMLElement | null {
const tabbables = getTabbableElements(container);
if (tabbables.length) {
tabbables[0].focus();
return tabbables[0];
}
if (!container.hasAttribute('tabindex')) {
container.setAttribute('tabindex', '-1');
}
container.focus();
return container;
}

export function handlePopupTabTrap(
e: React.KeyboardEvent,
container: HTMLElement,
): void {
if (e.key !== 'Tab' || e.defaultPrevented) {
return;
}

const list = getTabbableElements(container);
const active = document.activeElement as HTMLElement | null;

if (!active || !container.contains(active)) {
return;
}

if (list.length === 0) {
if (active === container) {
e.preventDefault();
}
return;
}

const first = list[0];
const last = list[list.length - 1];

if (!e.shiftKey) {
if (active === last || active === container) {
e.preventDefault();
first.focus();
}
} else if (active === first || active === container) {
e.preventDefault();
last.focus();
}
}
36 changes: 30 additions & 6 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,14 @@ export interface TriggerProps {
*/
unique?: boolean;

/**
* When true, moves focus into the popup on open (first tabbable node or the popup root with
* `tabIndex={-1}`), restores focus to the trigger on close, and keeps Tab cycling inside the
* popup. When undefined, enabled for click / contextMenu / focus triggers unless `hover` is also
* a show action (so hover-only tooltips are unchanged).
*/
focusPopup?: boolean;

// ==================== Arrow ====================
arrow?: boolean | ArrowTypeOuter;

Expand Down Expand Up @@ -211,6 +219,8 @@ export function generateTrigger(
// Private
mobile,

focusPopup: focusPopupProp,

...restProps
} = props;

Expand Down Expand Up @@ -331,6 +341,24 @@ export function generateTrigger(
// Support ref
const isOpen = useEvent(() => mergedOpen);

const [showActions, hideActions] = useAction(
action,
showAction,
hideAction,
);

const mergedFocusPopup = React.useMemo(() => {
if (focusPopupProp !== undefined) {
return focusPopupProp;
}
return (
!showActions.has('hover') &&
(showActions.has('click') ||
showActions.has('contextMenu') ||
showActions.has('focus'))
);
}, [focusPopupProp, showActions]);

// Extract common options for UniqueProvider
const getUniqueOptions = useEvent((delay: number = 0) => ({
popup,
Expand All @@ -354,6 +382,7 @@ export function generateTrigger(
getPopupClassNameFromAlign,
id,
onEsc,
focusPopup: mergedFocusPopup,
}));

// Handle controlled state changes for UniqueProvider
Expand Down Expand Up @@ -472,12 +501,6 @@ export function generateTrigger(
isMobile,
);

const [showActions, hideActions] = useAction(
action,
showAction,
hideAction,
);

const clickToShow = showActions.has('click');
const clickToHide =
hideActions.has('click') || hideActions.has('contextMenu');
Expand Down Expand Up @@ -838,6 +861,7 @@ export function generateTrigger(
autoDestroy={mergedAutoDestroy}
getPopupContainer={getPopupContainer}
onEsc={onEsc}
focusPopup={mergedFocusPopup}
// Arrow
align={alignInfo}
arrow={innerArrow}
Expand Down
Loading