Skip to content

feat(a11y): iframe accessibility and focus delegation - #1652

Open
AbhishekChorotiya wants to merge 1 commit into
feat/a11y-live-regionsfrom
feat/a11y-iframes
Open

feat(a11y): iframe accessibility and focus delegation#1652
AbhishekChorotiya wants to merge 1 commit into
feat/a11y-live-regionsfrom
feat/a11y-iframes

Conversation

@AbhishekChorotiya

@AbhishekChorotiya AbhishekChorotiya commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

This PR improves accessibility around embedded payment contexts. It centralizes the frame-title repair logic, removes duplicated frame-title handling, and improves focus delegation at embedded boundaries so keyboard users can move through the payment experience more predictably.

The user impact is clearer frame identification for assistive technology and better handoff when focus moves between merchant content and the embedded payment experience.

This PR is stacked after the live-region PR and should be reviewed after the announcement behavior is in place.

Closes #1651

How did you test it?

Validated as part of the completed accessibility stack. The checks cover the combined flow after all stacked PRs are applied, including embedded-frame behavior exercised during the local accessibility smoke checks.

  • Ran npm run re:build on the completed accessibility stack.
  • Ran npm run test:hooks on the completed accessibility stack.
  • Ran npm run build on the completed accessibility stack.
  • Ran committed-range whitespace validation on the completed accessibility stack.

Checklist

  • I ran npm run re:build
  • I reviewed submitted code
  • I added unit tests for my changes where possible

@semanticdiff-com

Copy link
Copy Markdown

Review changes with  SemanticDiff

@XyneSpaces

Copy link
Copy Markdown

💡 Focus delegation targetOrigin="*" should be constrained

The sendFocusNext and sendFocusPrevious functions in FocusDelegation.res use ~targetOrigin="*" which broadcasts to all origins. This could expose focus information to unintended recipients if the merchant page embeds multiple third-party frames.

// Current - broadcasts to all
let sendFocusNext = (~iframeId, ~targetOrigin="*") =>

// Recommended - use configured origin from GlobalVars
let sendFocusNext = (~iframeId, ~targetOrigin=GlobalVars.targetOrigin) =>

Impact: Low security risk (focus delegation is benign), but defense-in-depth suggests constraining the message target.

Descriptive title on every iframe; focus delegation across the iframe boundary via
postMessage (FocusDelegation reuses AccessibilityUtils focus/selector primitives).
@XyneSpaces

Copy link
Copy Markdown

Review Summary

PR: #1652 - feat(a11y): iframe accessibility and focus delegation

Findings

  1. Good: All iframe elements now include descriptive title attributes for screen reader context.

  2. Good: The FocusDelegation.res module implements proper cross-iframe focus management with sendFocusNext/sendFocusPrevious handlers.

  3. Issue: In AccessibilityUtils.res, the scheduleKnownIframeTitleRepair function uses multiple setTimeout calls (0ms, 1s, 3s) to repair iframe titles. This is a workaround pattern.

    Suggestion: Add a comment explaining why multiple attempts are needed and under what conditions this can be removed.

  4. Good: Integration with postFailedSubmitResponse includes accessibility announcements via announceFailedSubmit.

No blocking issues. The focus delegation implementation follows accessibility best practices.

[("focusDelegation", "next"->JSON.Encode.string), ("iframeId", iframeId->JSON.Encode.string)],
~targetOrigin,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Critical security concern - targetOrigin="*" in postMessage allows any origin to receive focus delegation messages.

While the message content (focus delegation) is relatively low-risk, this pattern establishes a precedent that could be copied for more sensitive messages. Consider deriving the target origin from the actual merchant origin or using a restrictive whitelist rather than the wildcard.

Suggested change
// Notify the parent page to move focus to the next focusable element after the iframe.
let sendFocusNext = (~iframeId, ~targetOrigin) =>
Utils.messageParentWindow(
[("focusDelegation", "next"->JSON.Encode.string), ("iframeId", iframeId->JSON.Encode.string)],
~targetOrigin,
)

@XyneSpaces

Copy link
Copy Markdown

🚨 Critical security concern - targetOrigin="*" in postMessage allows any origin to receive focus delegation messages

While the message content (focus delegation) is relatively low-risk, this pattern establishes a precedent that could be copied for more sensitive messages.

Fix: Derive the target origin from the actual merchant origin or use a restrictive whitelist:

// Instead of:
~targetOrigin="*"

// Use:
~targetOrigin=GlobalVars.hyperSwitchSdkUrl

Or iterate over allowed origins if multiple are supported.

Comment on lines +58 to +62
let scheduleKnownIframeTitleRepair = () => {
ensureKnownIframeTitles()
setTimeout(ensureKnownIframeTitles, 1000)->ignore
setTimeout(ensureKnownIframeTitles, 3000)->ignore
}

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.

why multi timeouts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants