From 1e2d0faa55dfca1b4e81bded56fadcf2a937471a Mon Sep 17 00:00:00 2001 From: madhushreeag Date: Tue, 31 Mar 2026 12:46:05 -0700 Subject: [PATCH] fix(dashboard): dashboard filters not inherited in charts in Safari sometimes due to race condition (#38851) Co-authored-by: madhushree agarwal --- superset-frontend/src/pages/Chart/index.tsx | 8 ++-- superset-frontend/src/utils/urlUtils.test.ts | 44 ++++++++++++++++++++ superset-frontend/src/utils/urlUtils.ts | 19 +++++++-- 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/pages/Chart/index.tsx b/superset-frontend/src/pages/Chart/index.tsx index 3f8d28ce2dc..46acccad36a 100644 --- a/superset-frontend/src/pages/Chart/index.tsx +++ b/superset-frontend/src/pages/Chart/index.tsx @@ -85,11 +85,11 @@ const getDashboardPageContext = (pageId?: string | null) => { return getItem(LocalStorageKeys.DashboardExploreContext, {})[pageId] || null; }; -const getDashboardContextFormData = () => { - const dashboardPageId = getUrlParam(URL_PARAMS.dashboardPageId); +const getDashboardContextFormData = (search: string) => { + const dashboardPageId = getUrlParam(URL_PARAMS.dashboardPageId, search); const dashboardContext = getDashboardPageContext(dashboardPageId); if (dashboardContext) { - const sliceId = getUrlParam(URL_PARAMS.sliceId) || 0; + const sliceId = getUrlParam(URL_PARAMS.sliceId, search) || 0; const { colorScheme, labelsColor, @@ -141,7 +141,7 @@ export default function ExplorePage() { fetchGeneration.current += 1; const generation = fetchGeneration.current; const exploreUrlParams = getParsedExploreURLParams(loc); - const dashboardContextFormData = getDashboardContextFormData(); + const dashboardContextFormData = getDashboardContextFormData(loc.search); const isStale = () => generation !== fetchGeneration.current; diff --git a/superset-frontend/src/utils/urlUtils.test.ts b/superset-frontend/src/utils/urlUtils.test.ts index e81d890d873..d995d539be4 100644 --- a/superset-frontend/src/utils/urlUtils.test.ts +++ b/superset-frontend/src/utils/urlUtils.test.ts @@ -22,7 +22,9 @@ import { parseUrl, toQueryString, getDashboardUrlParams, + getUrlParam, } from './urlUtils'; +import { URL_PARAMS } from '../constants'; test('isUrlExternal', () => { expect(isUrlExternal('http://google.com')).toBeTruthy(); @@ -145,3 +147,45 @@ test('getDashboardUrlParams should exclude multiple parameters when provided', ( // Restore original location window.location = originalLocation; }); + +test('getUrlParam reads from window.location.search by default', () => { + const originalLocation = window.location; + Object.defineProperty(window, 'location', { + value: { ...originalLocation, search: '?dashboard_page_id=from-window' }, + writable: true, + configurable: true, + }); + + expect(getUrlParam(URL_PARAMS.dashboardPageId)).toBe('from-window'); + + Object.defineProperty(window, 'location', { + value: originalLocation, + writable: true, + configurable: true, + }); +}); + +test('getUrlParam uses provided search string instead of window.location.search (Safari race condition fix)', () => { + // Simulate Safari race condition: window.location.search is stale (empty), + // but the correct search string is passed in from React Router's useLocation() + const originalLocation = window.location; + Object.defineProperty(window, 'location', { + value: { ...originalLocation, search: '' }, + writable: true, + configurable: true, + }); + + // Without the search override, window.location.search is stale — returns null (the bug) + expect(getUrlParam(URL_PARAMS.dashboardPageId)).toBeNull(); + + // With the search override (the fix), returns the correct value + expect( + getUrlParam(URL_PARAMS.dashboardPageId, '?dashboard_page_id=correct-id'), + ).toBe('correct-id'); + + Object.defineProperty(window, 'location', { + value: originalLocation, + writable: true, + configurable: true, + }); +}); diff --git a/superset-frontend/src/utils/urlUtils.ts b/superset-frontend/src/utils/urlUtils.ts index 4b218201144..dedc20c8330 100644 --- a/superset-frontend/src/utils/urlUtils.ts +++ b/superset-frontend/src/utils/urlUtils.ts @@ -38,22 +38,35 @@ export type UrlParamType = 'string' | 'number' | 'boolean' | 'object' | 'rison'; export type UrlParam = (typeof URL_PARAMS)[keyof typeof URL_PARAMS]; export function getUrlParam( param: UrlParam & { type: 'string' }, + search?: string, ): string | null; export function getUrlParam( param: UrlParam & { type: 'number' }, + search?: string, ): number | null; export function getUrlParam( param: UrlParam & { type: 'boolean' }, + search?: string, ): boolean | null; export function getUrlParam( param: UrlParam & { type: 'object' }, + search?: string, ): object | null; -export function getUrlParam(param: UrlParam & { type: 'rison' }): object | null; +export function getUrlParam( + param: UrlParam & { type: 'rison' }, + search?: string, +): string | object | null; export function getUrlParam( param: UrlParam & { type: 'rison | string' }, + search?: string, ): string | object | null; -export function getUrlParam({ name, type }: UrlParam): unknown { - const urlParam = new URLSearchParams(window.location.search).get(name); +export function getUrlParam( + { name, type }: UrlParam, + search?: string, +): unknown { + const urlParam = new URLSearchParams(search ?? window.location.search).get( + name, + ); switch (type) { case 'number': if (!urlParam) {