Skip to content

ref(browser)!: Drop Safari 14 support - remove navigationStart fallback and pagehide listeners #21293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
eddie333016 wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

ref(browser)!: Drop Safari 14 support - remove navigationStart fallback and pagehide listeners #21293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1f82e57
ref: Drop Safari 14 support - remove performance.timing.navigationSta…
eddie333016 Jun 2, 2026
66ba1a4
ref: Clean up stale navigationStart references in tests and fixtures
eddie333016 Jun 5, 2026
8bbc4b3
update unit test
msonnb Jul 22, 2026
284e5ea
test(e2e): Trigger INP flush via visibilitychange instead of pagehide
msonnb Jul 22, 2026
f3d6abc
remove `once: true`
msonnb Jul 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,8 @@ sentryTest(
envelopeType: 'span',
});

// Page hide to trigger INP

// Important: Purposefully not using hidePage() here to test the hidden state
// via the `pagehide` event. This is necessary because iOS Safari 14.4
// still doesn't fully emit the `visibilitychange` events but it's the lower
// bound for Safari on iOS that we support.
// If this test times out or fails, it's likely because we tried updating
// the web-vitals library which officially already dropped support for
// this iOS version
await page.evaluate(() => {
window.dispatchEvent(new Event('pagehide'));
});
// Page hide to trigger INP reporting
await hidePage(page);

// Get the INP span envelope
const span = (await spanPromise)[0];
Expand Down
3 changes: 2 additions & 1 deletion dev-packages/e2e-tests/test-applications/react-17/tests/transactions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ test('sends an INP span', async ({ page }) => {

// Page hide to trigger INP
await page.evaluate(() => {
window.dispatchEvent(new Event('pagehide'));
Object.defineProperty(document, 'visibilityState', { value: 'hidden', configurable: true });
document.dispatchEvent(new Event('visibilitychange'));
});

const inpSpan = await inpSpanPromise;
Expand Down
3 changes: 2 additions & 1 deletion dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ test('sends an INP span', async ({ page }) => {

// Page hide to trigger INP
await page.evaluate(() => {
window.dispatchEvent(new Event('pagehide'));
Object.defineProperty(document, 'visibilityState', { value: 'hidden', configurable: true });
document.dispatchEvent(new Event('visibilitychange'));
});

const inpSpan = await inpSpanPromise;
Expand Down
3 changes: 2 additions & 1 deletion dev-packages/e2e-tests/test-applications/react-router-7-spa-streaming/tests/spans.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ test('sends an INP span', async ({ page }) => {

// Page hide to trigger INP
await page.evaluate(() => {
window.dispatchEvent(new Event('pagehide'));
Object.defineProperty(document, 'visibilityState', { value: 'hidden', configurable: true });
document.dispatchEvent(new Event('visibilitychange'));
});

const inpSpan = await inpSpanPromise;
Expand Down
3 changes: 2 additions & 1 deletion dev-packages/e2e-tests/test-applications/react-router-7-spa/tests/transactions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ test('sends an INP span', async ({ page }) => {

// Page hide to trigger INP
await page.evaluate(() => {
window.dispatchEvent(new Event('pagehide'));
Object.defineProperty(document, 'visibilityState', { value: 'hidden', configurable: true });
document.dispatchEvent(new Event('visibilitychange'));
});

const inpSpan = await inpSpanPromise;
Expand Down
3 changes: 2 additions & 1 deletion dev-packages/e2e-tests/test-applications/react-router-8-spa/tests/transactions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ test('sends an INP span', async ({ page }) => {

// Page hide to trigger INP
await page.evaluate(() => {
window.dispatchEvent(new Event('pagehide'));
Object.defineProperty(document, 'visibilityState', { value: 'hidden', configurable: true });
document.dispatchEvent(new Event('visibilitychange'));
});

const inpSpan = await inpSpanPromise;
Expand Down
26 changes: 5 additions & 21 deletions packages/browser-utils/src/metrics/web-vitals/lib/getVisibilityWatcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@ const initHiddenTime = () => {

const onVisibilityUpdate = (event: Event) => {
// Handle changes to hidden state
if (isPageHidden(event) && firstHiddenTime > -1) {
// Sentry-specific change: Also call onHidden callbacks for pagehide events
// to support older browsers (Safari <14.4) that don't properly fire visibilitychange
if (event.type === 'visibilitychange' || event.type === 'pagehide') {
if (WINDOW.document?.visibilityState === 'hidden' && firstHiddenTime > -1) {
// Call onHidden callbacks when the page becomes hidden
if (event.type === 'visibilitychange') {
for (const onHiddenFunction of onHiddenFunctions) {
onHiddenFunction();
}
Expand All @@ -48,14 +47,14 @@ const onVisibilityUpdate = (event: Event) => {
// visible prior to this change, so the event timestamp is the first
// hidden time.
// However, if the event is not a 'visibilitychange' event, then it must
// be a 'prerenderingchange' or 'pagehide' event, and the fact that the document is
// be a 'prerenderingchange' event, and the fact that the document is
// still 'hidden' from the above check means the tab was activated
// in a background state and so has always been hidden.
firstHiddenTime = event.type === 'visibilitychange' ? event.timeStamp : 0;

// We no longer need the `prerenderingchange` event listener now we've
// set an initial init time so remove that
// (we'll keep the visibilitychange and pagehide ones for onHiddenFunction above)
// (we'll keep the visibilitychange one for onHiddenFunction above)
removePageListener('prerenderingchange', onVisibilityUpdate, true);
}
}
Expand All @@ -81,11 +80,6 @@ export const getVisibilityWatcher = () => {
// timestamps in detail and also for onHidden function calls.
addPageListener('visibilitychange', onVisibilityUpdate, true);

// Sentry-specific change: Some browsers have buggy implementations of visibilitychange,
// so we use pagehide in addition, just to be safe. This is also required for older
// Safari versions (<14.4) that we still support.
addPageListener('pagehide', onVisibilityUpdate, true);

// IMPORTANT: when a page is prerendering, its `visibilityState` is
// 'hidden', so in order to account for cases where this module checks for
// visibility during prerendering, an additional check after prerendering
Expand All @@ -102,13 +96,3 @@ export const getVisibilityWatcher = () => {
},
};
};

/**
* Check if the page is hidden, uses the `pagehide` event for older browsers support that we used to have in `onHidden` function.
* Some browsers we still support (Safari <14.4) don't fully support `visibilitychange`
* or have known bugs w.r.t the `visibilitychange` event.
* // TODO (v11): If we decide to drop support for Safari 14.4, we can use the logic from web-vitals 4.2.4
*/
function isPageHidden(event: Event) {
return event.type === 'pagehide' || WINDOW.document?.visibilityState === 'hidden';
}
29 changes: 7 additions & 22 deletions packages/browser-utils/src/metrics/web-vitals/lib/onHidden.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,36 +22,21 @@ export interface OnHiddenCallback {
}

/**
* Sentry-specific change:
* Calls the passed callback when the page transitions to hidden.
*
* This function's logic was NOT updated to web-vitals 4.2.4 or 5.x but we continue
* to use the web-vitals 3.5.2 version due to having stricter browser support.
* Uses the `visibilitychange` event exclusively, which is well-supported
* across all modern browsers.
*
* PR with context that made the changes:
* https://github.com/GoogleChrome/web-vitals/pull/442/files#r1530492402
*
* The PR removed listening to the `pagehide` event, in favour of only listening to
* the `visibilitychange` event. This is "more correct" but some browsers we still
* support (Safari <14.4) don't fully support `visibilitychange` or have known bugs
* with respect to the `visibilitychange` event.
*
* TODO (v11): If we decide to drop support for Safari 14.4, we can use the logic
* from web-vitals 4.2.4. In this case, we also need to update the integration tests
* that currently trigger the `pagehide` event to simulate the page being hidden.
*
* @param {OnHiddenCallback} cb - Callback to be executed when the page is hidden or unloaded.
* @param {OnHiddenCallback} cb - Callback to be executed when the page is hidden.
*
* @deprecated use `whenIdleOrHidden` or `addPageListener('visibilitychange')` instead
*/
export const onHidden = (cb: OnHiddenCallback) => {
const onHiddenOrPageHide = (event: Event) => {
if (event.type === 'pagehide' || WINDOW.document?.visibilityState === 'hidden') {
const onHiddenCallback = (event: Event) => {
if (WINDOW.document?.visibilityState === 'hidden') {
cb(event);
}
};

addPageListener('visibilitychange', onHiddenOrPageHide, { capture: true, once: true });
// Some browsers have buggy implementations of visibilitychange,
// so we use pagehide in addition, just to be safe.
addPageListener('pagehide', onHiddenOrPageHide, { capture: true, once: true });
addPageListener('visibilitychange', onHiddenCallback, { capture: true });
};
Comment thread
msonnb marked this conversation as resolved.
Comment on lines 39 to 42

@sentry sentry Bot Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The onHidden function adds a visibilitychange event listener without the { once: true } option and never removes it, causing a memory leak on every page load.
Severity: MEDIUM

Suggested Fix

Restore the { once: true } option to the addPageListener call within the onHidden function. This will ensure the event listener is automatically removed after it has been invoked once, preventing the memory leak and restoring the previous, correct behavior.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/browser-utils/src/metrics/web-vitals/lib/onHidden.ts#L39-L42

Potential issue: The `onHidden` function registers a `visibilitychange` event listener
using `addPageListener` but omits the `{ once: true }` option that was present in the
previous implementation. Since there is no corresponding call to `removePageListener`,
the listener is never garbage collected and persists for the entire page lifetime. This
function is called during the setup of `webVitalsIntegration` via `trackLcpAsSpan` and
`trackClsAsSpan`. Consequently, new, persistent listeners are created on every page
load, leading to a memory leak. While an internal guard prevents the callback from
executing more than once, the listener itself remains attached to the DOM.

@msonnb msonnb Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a page load cleans up all event listeners and the integration is only set up once, so this doesn't cause a memory leak. also matches web-vitals@4.2.4

7 changes: 0 additions & 7 deletions packages/browser-utils/src/metrics/web-vitals/lib/whenIdleOrHidden.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,11 @@ export const whenIdleOrHidden = (cb: () => void) => {
// eslint-disable-next-line no-param-reassign
cb = runOnce(cb);
addPageListener('visibilitychange', cb, { once: true, capture: true });
// sentry: we use pagehide instead of directly listening to visibilitychange
// because some browsers we still support (Safari <14.4) don't fully support
// `visibilitychange` or have known bugs w.r.t the `visibilitychange` event.
// TODO(v11): remove this once we drop support for Safari <14.4
addPageListener('pagehide', cb, { once: true, capture: true });
rIC(() => {
cb();
// Remove the above event listener since no longer required.
// See: https://github.com/GoogleChrome/web-vitals/issues/622
removePageListener('visibilitychange', cb, { capture: true });
// TODO(v11): remove this once we drop support for Safari <14.4
removePageListener('pagehide', cb, { capture: true });
});
}
};
3 changes: 2 additions & 1 deletion packages/browser/test/integrations/browsersession.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ describe('browserSessionIntegration', () => {
setupBrowserSession({ lifecycle: 'page' });
expect(SentryCore.captureSession).not.toHaveBeenCalled();

window.dispatchEvent(new Event('pagehide'));
setVisibilityState('hidden');
window.dispatchEvent(new Event('visibilitychange'));
expect(SentryCore.captureSession).toHaveBeenCalledTimes(1);

// The idle fallback must not send the session a second time.
Expand Down
26 changes: 4 additions & 22 deletions packages/core/src/utils/time.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ let cachedTimeOrigin: number | null | undefined = null;
/**
* Gets the time origin and the mode used to determine it.
*
* Unfortunately browsers may report an inaccurate time origin data, through either performance.timeOrigin or
* performance.timing.navigationStart, which results in poor results in performance data. We only treat time origin
* data as reliable if they are within a reasonable threshold of the current time.
* Unfortunately browsers may report inaccurate time origin data through performance.timeOrigin,
* which results in poor results in performance data. We only treat time origin data as reliable
* if it is within a reasonable threshold of the current time.
*
* TODO: move to `@sentry/browser-utils` package.
*/
Expand All @@ -104,25 +104,7 @@ function getBrowserTimeOrigin(): number | undefined {
}
}

// TODO: Remove all code related to `performance.timing.navigationStart` once we drop support for Safari 14.
// `performance.timeSince` is available in Safari 15.
// see: https://caniuse.com/mdn-api_performance_timeorigin

// While performance.timing.navigationStart is deprecated in favor of performance.timeOrigin, performance.timeOrigin
// is not as widely supported. Namely, performance.timeOrigin is undefined in Safari as of writing.
// Also as of writing, performance.timing is not available in Web Workers in mainstream browsers, so it is not always
// a valid fallback. In the absence of an initial time provided by the browser, fallback to the current time from the
// Date API.
// eslint-disable-next-line typescript/no-deprecated
const navigationStart = performance.timing?.navigationStart;
if (typeof navigationStart === 'number') {
const navigationStartDelta = Math.abs(navigationStart + performanceNow - dateNow);
if (navigationStartDelta < threshold) {
return navigationStart;
}
}
Comment on lines -117 to -123

@logaretm logaretm Jun 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some tests still assert the navigationStart. Those tests will fail.


// Either both timeOrigin and navigationStart are skewed or neither is available, fallback to subtracting
// timeOrigin is skewed or unavailable, fallback to subtracting
// `performance.now()` from `Date.now()`.
return dateNow - performanceNow;
}
Expand Down
33 changes: 1 addition & 32 deletions packages/core/test/lib/utils/time.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ describe('browserPerformanceTimeOrigin', () => {

vi.stubGlobal('performance', {
timeOrigin: unreliableTime,
timing: {
navigationStart: unreliableTime,
},
now: () => timeSincePageloadMs,
});

Expand All @@ -52,7 +49,7 @@ describe('browserPerformanceTimeOrigin', () => {
vi.unstubAllGlobals();
});

it('returns `Date.now() - performance.now()` if neither `performance.timeOrigin` nor `performance.timing.navigationStart` are available', async () => {
it('returns `Date.now() - performance.now()` if `performance.timeOrigin` is not available', async () => {
const currentTimeMs = 1767778040866;

const timeSincePageloadMs = 1_234.56789;
Expand All @@ -61,9 +58,6 @@ describe('browserPerformanceTimeOrigin', () => {
vi.setSystemTime(new Date(currentTimeMs));
vi.stubGlobal('performance', {
timeOrigin: undefined,
timing: {
navigationStart: undefined,
},
now: () => timeSincePageloadMs,
});

Expand All @@ -74,31 +68,6 @@ describe('browserPerformanceTimeOrigin', () => {
vi.unstubAllGlobals();
});

it('returns `performance.timing.navigationStart` if `performance.timeOrigin` is not available', async () => {
const currentTimeMs = 1767778040870;

const navigationStartMs = currentTimeMs - 2_000;

const timeSincePageloadMs = 1_234.789;

vi.useFakeTimers();
vi.setSystemTime(new Date(currentTimeMs));

vi.stubGlobal('performance', {
timeOrigin: undefined,
timing: {
navigationStart: navigationStartMs,
},
now: () => timeSincePageloadMs,
});

const timeOrigin = await getFreshPerformanceTimeOrigin();
expect(timeOrigin).toBe(navigationStartMs);

vi.useRealTimers();
vi.unstubAllGlobals();
});

describe('caching', () => {
it('caches `undefined` result', async () => {
vi.stubGlobal('performance', undefined);
Expand Down
2 changes: 1 addition & 1 deletion packages/replay-internal/test/fixtures/error.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export function Error(obj?: Event): any {
'plan.max_members': 'null',
'plan.total_members': '15',
'plan.tier': 'am1',
'timeOrigin.mode': 'navigationStart',
'timeOrigin.mode': 'timeOrigin',
}, // }}}
user: {
ip_address: '0.0.0.0',
Expand Down
2 changes: 1 addition & 1 deletion packages/replay-internal/test/fixtures/transaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export function Transaction(traceId?: string, obj?: Partial<Event>): any {
deviceMemory: '8 GB',
hardwareConcurrency: '10',
sentry_reportAllChanges: false,
'timeOrigin.mode': 'navigationStart',
'timeOrigin.mode': 'timeOrigin',
},
transaction: '/organizations/:orgId/replays/:replaySlug/',
type: 'transaction' as const,
Expand Down
Loading