mikebridge opened a new pull request, #43830:
URL: https://github.com/apache/superset/pull/43830
### SUMMARY
The version-history panel's **"Open as new chart / Open as new dashboard"**
(and the related-entity open) opened a **blank `about:blank` tab** instead of
the forked object — reproduced for both current and older versions, on charts
and dashboards.
Root cause is in `superset-frontend/src/utils/navigationUtils.ts`. These
flows follow a *claim-then-navigate* pattern: because the fork takes several
sequential requests (snapshot → resolve → copy), they call `openBlankTab()`
**synchronously in the click handler** — while the click's transient user
activation is still live — and then point that tab at the destination once the
new object's id is known, via `navigateOpenedTab()`. This avoids the popup
blocker refusing a `window.open` issued after the awaits.
But `openBlankTab()` opened its placeholder with `window.open('', '_blank',
'noopener noreferrer')`, and **per the HTML standard `window.open(...,
'noopener')` always returns `null`** — that is the entire purpose of
`noopener`: sever the opener link, so the caller gets no window handle. So the
handle the function exists to return was discarded on every call:
1. `openBlankTab()` opens a blank tab but returns `null` → `tab = null`.
2. The fork runs (snapshot fetch, uuid resolve, `POST /copy/` or `POST
/chart/`) — all succeed; the server creates the new object.
3. `navigateOpenedTab(null, url)` sees a null handle and falls through to a
**second** `window.open(url, ...)`, which by now has lost user activation and
is silently refused by the popup blocker.
4. The blank tab from step 1 is stranded on `about:blank`.
The fix opens the placeholder **without** `noopener` so the returned handle
is usable, and `navigateOpenedTab` can call `tab.location.replace(url)` on the
live tab. The destination is always a **same-origin app route** (built through
`ensureAppRoot` and validated by `assertSafeNavigationUrl`), so the opener
relationship carries no cross-origin tabnabbing risk. The one-shot
`window.open(url, ...)` fallback in `navigateOpenedTab` keeps `noopener`, since
it passes the real URL directly and never needs the handle.
Scope: this fixes every claim-then-navigate path — chart open-as-new,
dashboard open-as-new, and `openRelatedEntity` (all share `openBlankTab` /
`navigateOpenedTab`). In-place **Preview** was never affected because it
renders in the panel and opens no tab.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before:** selecting "Open as new chart/dashboard" from the version-history
kebab opens a new tab that stays on `about:blank` — even though the fork
succeeded on the server (the copy request returns `200` with the new id).
Verified live: `POST /api/v1/dashboard/<id>/copy/` → `200 {"result":{"id":N}}`,
and navigating directly to `/dashboard/N/` renders the fork correctly — only
the automatic tab navigation was broken.
**After:** the claimed tab receives a real window handle and is navigated to
the new object's route (`/explore/?slice_id=N` or `/dashboard/N/`), so the
forked chart/dashboard opens populated, as intended.
### TESTING INSTRUCTIONS
Requires the versioning UI (`SOFT_DELETE` / version-history feature) enabled
and an object with at least one saved version.
1. Open a chart in Explore (or a dashboard) → 3-dot menu → **View version
history**.
2. On a version's kebab → **Open as new chart** / **Open as new dashboard**.
3. **Before this fix:** a new tab opens and stays blank. **After:** the new
tab opens the forked chart/dashboard, populated from that version.
4. Repeat for the current version and an older version.
Automated: `cd superset-frontend && npx jest
src/utils/navigationUtils.test.ts` — adds coverage for `openBlankTab` (returns
a usable handle, no `noopener`), `navigateOpenedTab` (live-handle `replace`
with app-root prefix, URL validation before touching the tab, null/closed →
`window.open` fallback), and `closeOpenedTab`.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
