Skip to content

fix: incorrect nested iframe xpaths #891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 18, 2025

Conversation

seanmcguire12
Copy link
Member

@seanmcguire12 seanmcguire12 commented Jul 17, 2025

why

  • incorrect/broken xpaths for nested iframes with siblings meant act often failed on websites where the HTML follows this pattern
  • getAccessibilityTreeWithFrames() populates an internal map: seg: Map<Frame | null, string>
  • for every iframe it visits, the map stores the frameXpath the XPath segment (/html/body/iframe[2], …) pointing from that iframe’s parent document to the iframe element itself
  • previously, the map key was the parent frame: seg.set(snapshot.parentFrame, snapshot.frameXpath);
  • if a website contains 2+ sibling iframes, the first siblings frameXpath would get overwritten/lost

what changed

  • keyed the seg map by the frame itself so that siblings are no longer overwritten

test plan

  • added an eval that has nested iframes with siblings

Copy link

changeset-bot bot commented Jul 17, 2025

🦋 Changeset detected

Latest commit: 7b5d562

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@seanmcguire12 seanmcguire12 marked this pull request as ready for review July 17, 2025 21:44
@seanmcguire12 seanmcguire12 added act These changes pertain to the act function extract These changes pertain to the extract function labels Jul 17, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR fixes a critical bug in the XPath generation system for nested iframes. The core issue was that when multiple sibling iframes existed on a page, the XPath mapping would incorrectly overwrite previous siblings' paths, causing act() operations to fail. This was due to using the parent frame as the key in the internal seg map.

The fix changes the seg map to be keyed by the frame itself rather than its parent frame, ensuring each iframe's XPath information is preserved independently. This is a robust solution that maintains the integrity of the XPath hierarchy while fixing the sibling iframe issue.

The changes include:

  • Modified lib/a11y/utils.ts to use frame instead of parentFrame as map key
  • Added a new required frame field to FrameSnapshot interface
  • Added a new eval test case 'nested_iframes_2' to validate the fix

Confidence score: 5/5

  1. This PR is very safe to merge as it fixes a clear bug without introducing new complexity
  2. The score is 5 because the change is well-isolated, includes tests, and fixes a clear logical error in the original implementation
  3. While all changes look good, pay special attention to lib/a11y/utils.ts as it contains the core fix

4 files reviewed, no comments
Edit PR Review Bot Settings | Greptile

@MrHaikuSwan
Copy link

@seanmcguire12 = 🐐

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
act These changes pertain to the act function extract These changes pertain to the extract function
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants