This is an automated email from the ASF dual-hosted git repository.
eschutho pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 86ba63b0723 fix(dashboard): prevent duplicate subdirectory prefix when
toggling fullscreen (#39534)
86ba63b0723 is described below
commit 86ba63b0723627c9cd18d7cb699e4b21598619a1
Author: Elizabeth Thompson <[email protected]>
AuthorDate: Tue May 12 16:51:37 2026 -0700
fix(dashboard): prevent duplicate subdirectory prefix when toggling
fullscreen (#39534)
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
---
.../DashboardBuilder/DashboardBuilder.test.tsx | 3 +
.../dashboard/components/Header/Header.test.tsx | 91 ++++++++++++++++++++++
.../Header/useHeaderActionsDropdownMenu.tsx | 13 +++-
.../src/dashboard/util/getDashboardUrl.test.ts | 10 +++
superset-frontend/src/utils/pathUtils.test.ts | 16 ++++
superset-frontend/src/utils/pathUtils.ts | 10 ++-
6 files changed, 140 insertions(+), 3 deletions(-)
diff --git
a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx
b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx
index 6fb5c0e790b..3d368f64890 100644
---
a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx
+++
b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx
@@ -139,6 +139,7 @@ describe('DashboardBuilder', () => {
...overrideState,
}),
useDnd: true,
+ useRouter: true,
useTheme: true,
});
}
@@ -473,6 +474,7 @@ test('should render ParentSize wrapper with height 100% for
tabs', async () => {
dashboardLayout: undoableDashboardLayoutWithTabs,
}),
useDnd: true,
+ useRouter: true,
useTheme: true,
});
@@ -506,6 +508,7 @@ test('should maintain layout when switching between tabs',
async () => {
dashboardLayout: undoableDashboardLayoutWithTabs,
}),
useDnd: true,
+ useRouter: true,
useTheme: true,
});
diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx
b/superset-frontend/src/dashboard/components/Header/Header.test.tsx
index 6829c8fe853..ac331442021 100644
--- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx
+++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx
@@ -31,6 +31,20 @@ import { DASHBOARD_HEADER_ID } from '../../util/constants';
import { UPDATE_COMPONENTS } from '../../actions/dashboardLayout';
import { AutoRefreshStatus } from '../../types/autoRefresh';
+const mockHistoryReplace = jest.fn();
+jest.mock('react-router-dom', () => ({
+ ...jest.requireActual('react-router-dom'),
+ useHistory: () => ({
+ replace: mockHistoryReplace,
+ }),
+ useLocation: jest.fn(() => ({
+ pathname: '/dashboard',
+ search: '?standalone=1',
+ hash: '',
+ state: undefined,
+ })),
+}));
+
const initialState = {
dashboardInfo: {
id: 1,
@@ -223,6 +237,13 @@ beforeAll(() => {
beforeEach(() => {
jest.clearAllMocks();
+ const { useLocation } = jest.requireMock('react-router-dom');
+ useLocation.mockReturnValue({
+ pathname: '/dashboard',
+ search: '?standalone=1',
+ hash: '',
+ state: undefined,
+ });
(useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
showModal: false,
@@ -984,3 +1005,73 @@ test('should sync theme ref when navigating between
dashboards', async () => {
expect(setUnsavedChanges).toHaveBeenCalledTimes(0);
});
});
+
+test('should not duplicate subdirectory prefix when toggling fullscreen',
async () => {
+ const { useLocation } = jest.requireMock('react-router-dom');
+ // Simulate React Router with basename=/pcs: useLocation returns path
relative to basename
+ useLocation.mockReturnValue({
+ pathname: '/dashboard',
+ search: '?standalone=1',
+ hash: '',
+ state: undefined,
+ });
+ // Simulate browser URL including the subdirectory prefix
+ window.history.pushState({}, 'Test page', '/pcs/dashboard?standalone=1');
+
+ setup();
+ await openActionsDropdown();
+ userEvent.click(screen.getByText('Exit fullscreen'));
+
+ // history.replace must be called with the Router-relative path, not
window.location.pathname.
+ // If the subdirectory prefix (/pcs) were included, React Router would
prepend it again,
+ // producing /pcs/pcs/dashboard (the bug). The path must start with
/dashboard, not /pcs/.
+ expect(mockHistoryReplace).toHaveBeenCalledWith(
+ expect.not.stringMatching(/^\/pcs\//),
+ );
+ expect(mockHistoryReplace).toHaveBeenCalledWith(
+ expect.stringMatching(/^\/dashboard(\?|$)/),
+ );
+});
+
+test('should not duplicate subdirectory prefix when entering fullscreen',
async () => {
+ const { useLocation } = jest.requireMock('react-router-dom');
+ useLocation.mockReturnValue({
+ pathname: '/dashboard',
+ search: '',
+ hash: '',
+ state: undefined,
+ });
+ window.history.pushState({}, 'Test page', '/pcs/dashboard');
+
+ setup();
+ await openActionsDropdown();
+ userEvent.click(screen.getByText('Enter fullscreen'));
+
+ expect(mockHistoryReplace).toHaveBeenCalledWith(
+ expect.not.stringMatching(/^\/pcs\//),
+ );
+ expect(mockHistoryReplace).toHaveBeenCalledWith(
+ expect.stringMatching(/^\/dashboard\?standalone=1$/),
+ );
+});
+
+test('share URL should use browser-absolute pathname to preserve subdirectory
prefix', () => {
+ const { useLocation } = jest.requireMock('react-router-dom');
+ // Router returns path without the subdirectory prefix
+ useLocation.mockReturnValue({
+ pathname: '/dashboard',
+ search: '',
+ hash: '',
+ state: undefined,
+ });
+ // Browser URL includes the full prefix
+ window.history.pushState({}, 'Test page', '/pcs/dashboard');
+
+ const { container } = setup();
+ // The share/embed URL must use window.location.pathname so that shared links
+ // include the subdirectory prefix and work outside the React Router context.
+ const emailLink = container.querySelector('[data-test="share-by-email"]');
+ if (emailLink) {
+ expect(emailLink.getAttribute('href')).toMatch(/\/pcs\/dashboard/);
+ }
+});
diff --git
a/superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx
b/superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx
index 121759846c8..102a0a70581 100644
---
a/superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx
+++
b/superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx
@@ -19,7 +19,7 @@
import type { Dispatch, ReactElement, SetStateAction } from 'react';
import { useState, useEffect, useCallback, useMemo } from 'react';
import { useSelector } from 'react-redux';
-import { useHistory } from 'react-router-dom';
+import { useHistory, useLocation } from 'react-router-dom';
import { Menu, MenuItem } from '@superset-ui/core/components/Menu';
import { t } from '@apache-superset/core/translation';
import { isEmpty } from 'lodash';
@@ -75,6 +75,7 @@ export const useHeaderActionsMenu = ({
const [isDropdownVisible, setIsDropdownVisible] = useState(false);
const { canExportImage } = usePermissions();
const history = useHistory();
+ const location = useLocation();
const directPathToChild = useSelector(
(state: RootState) => state.dashboardState.directPathToChild,
);
@@ -101,8 +102,11 @@ export const useHeaderActionsMenu = ({
case MenuKeys.ToggleFullscreen: {
const isCurrentlyStandalone =
Number(getUrlParam(URL_PARAMS.standalone)) === 1;
+ // Use location.pathname from React Router (relative to basename)
rather than
+ // window.location.pathname to avoid duplicating the subdirectory
prefix when
+ // history.replace prepends it again.
const url = getDashboardUrl({
- pathname: window.location.pathname,
+ pathname: location.pathname,
filters: getActiveFilters(),
hash: window.location.hash,
standalone: isCurrentlyStandalone ? null : 1,
@@ -125,6 +129,7 @@ export const useHeaderActionsMenu = ({
showRefreshModal,
manageEmbedded,
history,
+ location,
],
);
@@ -133,6 +138,10 @@ export const useHeaderActionsMenu = ({
[dashboardTitle],
);
+ // window.location.pathname is intentional here: this URL is used for sharing
+ // (email, embed, copy link) and must be a full browser-absolute path that
+ // includes the application root. Do NOT replace with useLocation().pathname
—
+ // that would strip the subdirectory prefix and produce a broken share link.
const url = useMemo(
() =>
getDashboardUrl({
diff --git a/superset-frontend/src/dashboard/util/getDashboardUrl.test.ts
b/superset-frontend/src/dashboard/util/getDashboardUrl.test.ts
index c68caafe2e1..530d6a8ed06 100644
--- a/superset-frontend/src/dashboard/util/getDashboardUrl.test.ts
+++ b/superset-frontend/src/dashboard/util/getDashboardUrl.test.ts
@@ -115,6 +115,16 @@ describe('getChartIdsFromLayout', () => {
windowSpy.mockRestore();
});
+ test('should pass through a router-relative pathname unchanged', () => {
+ const url = getDashboardUrl({
+ pathname: '/dashboard/1/',
+ filters: {},
+ hash: '',
+ standalone: DashboardStandaloneMode.HideNav,
+ });
+
expect(url).toBe(`/dashboard/1/?standalone=${DashboardStandaloneMode.HideNav}`);
+ });
+
test('should process native filters key', () => {
const windowSpy = jest.spyOn(window, 'window', 'get');
windowSpy.mockImplementation(
diff --git a/superset-frontend/src/utils/pathUtils.test.ts
b/superset-frontend/src/utils/pathUtils.test.ts
index fd546ff01d5..f89aa01ed1a 100644
--- a/superset-frontend/src/utils/pathUtils.test.ts
+++ b/superset-frontend/src/utils/pathUtils.test.ts
@@ -229,3 +229,19 @@ test('ensureAppRoot should prefix unknown schemes instead
of passing through', a
// Unknown / custom schemes are treated as relative paths
expect(ensureAppRoot('foo:bar')).toBe('/superset/foo:bar');
});
+
+test('ensureAppRoot should be idempotent — not double-prefix an
already-prefixed path', async () => {
+ const { ensureAppRoot } = await loadPathUtils('/superset/');
+
+ const once = ensureAppRoot('/sqllab');
+ const twice = ensureAppRoot(once);
+ expect(twice).toBe(once); // /superset/sqllab, NOT /superset/superset/sqllab
+});
+
+test('makeUrl should be idempotent with subdirectory prefix', async () => {
+ const { makeUrl } = await loadPathUtils('/superset/');
+
+ const once = makeUrl('/sqllab?new=true');
+ const twice = makeUrl(once);
+ expect(twice).toBe(once); // /superset/sqllab?new=true, NOT
/superset/superset/sqllab?new=true
+});
diff --git a/superset-frontend/src/utils/pathUtils.ts
b/superset-frontend/src/utils/pathUtils.ts
index 870d8aa95ab..477766e1313 100644
--- a/superset-frontend/src/utils/pathUtils.ts
+++ b/superset-frontend/src/utils/pathUtils.ts
@@ -41,7 +41,15 @@ export function ensureAppRoot(path: string): string {
if (SAFE_ABSOLUTE_URL_RE.test(path) || path.startsWith('//')) {
return path;
}
- return `${applicationRoot()}${path.startsWith('/') ? path : `/${path}`}`;
+ const root = applicationRoot();
+ const normalizedPath = path.startsWith('/') ? path : `/${path}`;
+ if (
+ root &&
+ (normalizedPath === root || normalizedPath.startsWith(`${root}/`))
+ ) {
+ return normalizedPath;
+ }
+ return `${root}${normalizedPath}`;
}
/**
