fix: resolve iframe synchronization timing issue between load and click events

[Raakshass]

summary

takes ownership of #141 (original author @hahahannes, inactive since August 2025). re-applied the fix on current main with failing unit tests fixed.

closes #65

root cause

the iframe-container's load event listener attaches click and hashchange handlers but does not set this.page from the loaded iframe's current location. when a non-client-side navigation occurs inside the iframe (server-side redirect / GET request), the click event fires before the page property is initialized, causing the parent URL to either not update or update to a stale value.

additionally, _iframePageChanged in main-page.js calls this.set('routeHash.path', ...) after window.history.replaceState. this triggers the _routePageChanged observer a second time, creating a re-entrant state loop that produces incorrect URL synchronization.

fix

1. iframe-container.js: set this.page from the iframe's location on the load event, before attaching click/hashchange listeners. this ensures the page property is always initialized to the current iframe location after navigation.

2. main-page.js: remove the redundant this.set('routeHash.path', ...) call. window.history.replaceState already updates the URL. the hash is reflected through the load event sync, making the explicit set unnecessary and harmful.

3. iframe-container_test.js: remove two expect(page).toBe(undefined) assertions. with the load-time sync, page is no longer undefined after beforeEach completes — it is set to the loaded iframe's path. the remaining assertions (click and hashchange update page to the expected value) are preserved.

changes

file	change
components/centraldashboard/public/components/iframe-container.js	set this.page from iframe location on load event
components/centraldashboard/public/components/main-page.js	remove re-entrant routeHash.path set in _iframePageChanged
components/centraldashboard/public/components/iframe-container_test.js	remove stale undefined assertions for page property

what is not changed

• click and hashchange synchronization logic unchanged
• namespace message passing unchanged
• all other tests unmodified

cc @juliusvonkohout @andyatmiami @thesuperzapper

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign thesuperzapper for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• components/centraldashboard/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Copilot]

Pull request overview

Fixes iframe URL synchronization between parent dashboard and embedded iframe content by addressing a timing issue where this.page was not initialized on iframe load, and removing a re-entrant hash update that caused incorrect URL state.

Changes:

• Initialize this.page from the iframe's current location inside the load event handler before attaching click/hashchange listeners.
• Remove the redundant this.set('routeHash.path', ...) call after window.history.replaceState in _iframePageChanged.
• Drop two now-invalid expect(page).toBe(undefined) assertions in the iframe-container tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
components/centraldashboard/public/components/iframe-container.js	Set this.page from the iframe's location on each load event
components/centraldashboard/public/components/main-page.js	Remove re-entrant routeHash.path set causing stale URL updates
components/centraldashboard/public/components/iframe-container_test.js	Remove stale undefined assertions invalidated by load-time page init

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[andyatmiami]

/hold

Can you resolve/address this comment from the original PR - at which point we could consider re-reviewing and approving this:

• fix: iframe synchronization #141 (comment)

[Raakshass]

@andyatmiami addressing your request from #141 (comment).
deployed the fix on a local kind cluster with istio ingress and verified both bugs against the same environment:
before (v1.10.0):

• _iframePageChanged contains re-entrant routeHash.path set → true (bug present)
• load event handler does not initialize this.page → false (bug present)
• iframe reload produces RESULT: page is STALE
  after (this PR):
• re-entrant routeHash.path set removed → false (fixed)
• load handler reads event.target.contentWindow.location and sets this.page → true (fixed)
  full walkthrough video: https://www.youtube.com/watch?v=Bz4zd1hMRFU
  all CI checks pass (unit tests, integration test, multi-arch build, e2e, DCO).