Skip to content

Iframe offset walk terminates at window.top instead of Shepherd's own window #3478

Description

@chuckcarpenter

Split out of #1692, where it was noticed while answering the same-origin iframe question. Not reproduced — this is inferred from reading the code, so the first job is to confirm or kill it.

The suspicion

_getIframeOffset in shepherd.js/src/components/shepherd-modal.ts walks the frame chain like this:

let targetWindow: Window | null = el.ownerDocument.defaultView;

while (targetWindow && targetWindow !== window.top) {
  const targetIframe = targetWindow?.frameElement;
  if (targetIframe) {
    const rect = targetIframe.getBoundingClientRect();
    offset.top += rect.top + targetIframe.scrollTop;
    offset.left += rect.left + targetIframe.scrollLeft;
  }
  targetWindow = targetWindow.parent;
}

The loop terminates at window.top — the outermost frame. But the offsets are applied to the modal overlay, which is position: fixed inside the document Shepherd itself renders into, not necessarily the top one.

So when Shepherd runs inside a same-origin iframe and targets an element in a frame nested deeper, the walk should stop at Shepherd's own window. Terminating at window.top looks like it keeps accumulating past that point, adding the intermediate frame's offset to a cutout that is already positioned relative to it — pushing the overlay opening off the target by roughly that frame's position.

The straightforward case (Shepherd in the top document, target one iframe down) is unaffected, since there window.top is Shepherd's window. That is also the only case the tests cover.

Why it wasn't caught

The existing tests in shepherd.js/test/unit/components/shepherd-modal.spec.js mock the frame chain in jsdom but only assert that the modal became visible and that cross-origin traversal doesn't throw. Neither asserts the resulting offset values, so any arithmetic error passes.

Suggested work

  1. Build a real two-level same-origin repro (top → frame A hosting Shepherd → frame B holding the target) and confirm or refute the drift.
  2. If confirmed, terminate the walk at the window owning the modal container rather than window.top. Tour already knows its container (shepherd.js/src/tour.ts), so the owning window is reachable.
  3. Add tests that assert the computed offset numbers, not just visibility. That gap is the reason this went unnoticed.

Cross-origin nesting stays out of scope and unsupported — see #1692 and #3087.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions