fix: use legacy Safari push path only for existing legacy subscribers by sherwinski · Pull Request #1441 · OneSignal/OneSignal-Website-SDK

sherwinski · 2026-03-20T04:57:21Z

Description

1 Line Summary

Fix Safari 16.4+ to use VAPID push for new users instead of the broken legacy path, while preserving legacy push for existing subscribers and Safari < 16.4.

Details

useSafariLegacyPush() previously returned true whenever window.safari.pushNotification existed, forcing ALL macOS Safari users onto the legacy push path — including Safari 16.4+ which fully supports standard Web Push (VAPID). This caused new users on modern Safari to hit "Safari url/icon/certificate invalid" errors.

Now the function checks the actual subscription state:

Safari 16.4+: Only users with an active legacy subscription (permission === 'granted' with a valid deviceToken) stay on the legacy path. New users go through VAPID.
Safari < 16.4: PushSubscriptionOptions is unavailable, so these browsers are routed to the legacy path regardless of subscription state.

Scenario	Before	After
New user on Safari 16.4+	Legacy path (broken)	VAPID path
Existing legacy subscriber on Safari 16.4+	Legacy path	Legacy path (preserved)
Any user on Safari < 16.4	Legacy path	Legacy path
Chrome	Unaffected	Unaffected

Systems Affected

WebSDK
Backend
Dashboard

Validation

Tests

Info

12 new unit tests covering useSafariLegacyPush() and getSubscriptionType() across Safari < 16.4, Safari 16.4+ new users, and existing legacy subscribers.

Manual testing performed on:

Safari 15.6 (BrowserStack Local): New user subscribes via legacy path successfully
Safari 26.2 (local): New user with clean profile gets VAPID slidedown, subscribes with web.push.apple.com token, type = SafariPush
Safari 26.2 (local): Existing legacy subscriber (created on main) correctly detected and kept on legacy path with deviceToken preserved
Chrome 146 (local): No regression, standard VAPID flow works

Checklist

All the automated tests pass or I explained why that is not possible
I have personally tested this on my machine or explained why that is not possible
I have included test coverage for these changes or explained why they are not needed

Programming Checklist
Interfaces:

Don't use default export
New interfaces are in model files

Functions:

Don't use default export
All function signatures have return types
Helpers should not access any data but rather be given the data to operate on.

Typescript:

No Typescript warnings
Avoid silencing null/undefined warnings with the exclamation point

Other:

Iteration: refrain from using elem of array syntax. Prefer forEach or use map
Avoid using global OneSignal accessor for context if possible. Instead, we can pass it to function/constructor so that we don't call OneSignal.context

Screenshots

Info

Safari 15.6 — new user, legacy path (BrowserStack)

Safari 26.2 — new user, VAPID path

Safari 26.2 — existing legacy subscriber preserved

Chrome 146 — no regression

Checklist

I have included screenshots/recordings of the intended results or explained why they are not needed

sherwinski · 2026-03-31T15:11:29Z

@fadi-george @abdulraqeeb33 Results of manually-tested scenarios:

Scenario	Browser	Path Taken	Result
New user	Safari 15.6 (< 16.4)	Legacy (no VAPID available)	Pass
New user (clean slate)	Safari 26.2 (16.4+)	VAPID	Pass
Existing legacy subscriber	Safari 26.2 (16.4+)	Legacy (preserved)	Pass
Existing subscriber	Chrome 146	VAPID (Chrome)	Pass

Safari 16.4+ supports standard Web Push (VAPID), but `window.safari.pushNotification` was never removed. The SDK checked for API existence to decide the push path, which routed ALL macOS Safari users through the legacy flow — even on versions that fully support VAPID. When the legacy push package validation fails (e.g. invalid allowedDomains), this causes the error: "Safari url/icon/certificate invalid or in private mode" Now `useSafariLegacyPush()` checks the actual permission state: only users who have already granted legacy push (with a valid deviceToken) stay on the legacy path. New users on Safari 16.4+ are routed through the standard VAPID flow via `useSafariVapidPush()`.

Cover the new `useSafariLegacyPush()` behavior: - Returns false when window.safari is undefined - Returns false when pushNotification API exists but no safariWebId - Returns false for new users (permission "default", no deviceToken) - Returns false for denied users - Returns true only for existing legacy subscribers (granted + deviceToken) Also tests `getSubscriptionType()` integration to verify legacy subscribers stay on SafariLegacyPush while new users are not routed to the legacy path.

For Safari < 16.4 where VAPID isn't available, returns early so users are routed through the legacy Safari push path. This is the only path that works on those browsers. On Safari 16.4+ the existing logic still applies: only users with an active legacy subscription (granted + deviceToken) use the legacy path.

Existing tests now explicitly mock VAPID support in beforeEach to simulate Safari 16.4+ Adds two new test cases for Safari < 16.4: - useSafariLegacyPush returns true even for new users when VAPID isn't available - getSubscriptionType returns _SafariLegacyPush when VAPID isn't available

sherwinski · 2026-03-31T15:25:54Z

@claude[agent] review

Claude · 2026-03-31T15:27:46Z

@claude[agent] review

Code review complete. The implementation correctly fixes Safari 16.4+ VAPID routing by checking permission state instead of just API presence. Test coverage is comprehensive (10 new tests), manual testing validates all scenarios, and backward compatibility is preserved for existing legacy subscribers. Minor suggestion: consider reusing supportsVapidPush at detect.ts:41-43 to eliminate duplication. Overall: LGTM ✅

fadi-george · 2026-03-31T17:39:14Z

+  };
+};
+
+const clearVapidSupport = () => {


doesnt seem needed, tests pass without it

This is what I get:

FAIL src/shared/environment/detect.test.ts > getSubscriptionType > returns SafariLegacyPush on Safari < 16.4 (no VAPID support) AssertionError: expected 'ChromePush' to be 'SafariLegacyPush' // Object.is equality Expected: "SafariLegacyPush" Received: "ChromePush"

fadi-george · 2026-03-31T17:43:27Z

    "prepare": "vp config"
  },
  "dependencies": {
+    "@types/jest": "^30.0.0",


At the time it was needed for changes to the test files; specifically for importing afterEach and beforeEach

what do you mean, that comes from vitest

ran build,test, & lint without it and didnt get any errors

Shoot, I didn't realize this was added as a dependency. Removing

fadi-george · 2026-03-31T17:45:07Z

but even legacy safari subscriptions on 16.4+ can be migrated, see
#1329
but could be a follow up pr

fadi-george · 2026-03-31T17:47:29Z

@@ -150,10 +150,10 @@ export default defineConfig({
  server: {


maybe could do:

const getBooleanEnv = (env?: string) => { return !!env && env === 'true'; };

abdulraqeeb33 · 2026-03-31T18:25:41Z

@fadi-george @abdulraqeeb33 Results of manually-tested scenarios:

Scenario Browser Path Taken Result
New user Safari 15.6 (< 16.4) Legacy (no VAPID available) Pass
New user (clean slate) Safari 26.2 (16.4+) VAPID Pass
Existing legacy subscriber Safari 26.2 (16.4+) Legacy (preserved) Pass
Existing subscriber Chrome 146 VAPID (Chrome) Pass

do we have to test on any other browser?
do we claim we support this on other browsers?

sherwinski · 2026-03-31T18:50:25Z

but even legacy safari subscriptions on 16.4+ can be migrated, see #1329 but could be a follow up pr

Agreed, let's have the next release address:

token migration from legacy to VAPID
drop support for legacy subscriptions

sherwinski · 2026-03-31T18:51:19Z

@fadi-george @abdulraqeeb33 Results of manually-tested scenarios:
Scenario Browser Path Taken Result
New user Safari 15.6 (< 16.4) Legacy (no VAPID available) Pass
New user (clean slate) Safari 26.2 (16.4+) VAPID Pass
Existing legacy subscriber Safari 26.2 (16.4+) Legacy (preserved) Pass
Existing subscriber Chrome 146 VAPID (Chrome) Pass

do we have to test on any other browser? do we claim we support this on other browsers?

I included a test for Chrome to show there were no regressions. The issue that this PR addresses only affects Safari.

fadi-george · 2026-03-31T21:14:45Z

+  const safariWebId = OneSignal?.config?.safariWebId;
+  if (!safariWebId) return false;
+
+  const hasVapidSupport =


We already do this in supportsVapidPush so could do:

export const supportsVapidPush = typeof PushSubscriptionOptions !== 'undefined' && // eslint-disable-next-line no-prototype-builtins PushSubscriptionOptions.prototype.hasOwnProperty('applicationServerKey'); export const useSafariLegacyPush = (): boolean => { if (!isBrowser() || window.safari?.pushNotification == undefined) { return false; } const safariWebId = OneSignal?.config?.safariWebId; if (!safariWebId) return false; if (!supportsVapidPush) return true; const { permission, deviceToken } = window.safari.pushNotification.permission(safariWebId); return permission === 'granted' && deviceToken != null; };

sherwinski force-pushed the fix/safari-vapid-push-detection branch from 2c6fed0 to 034ccf0 Compare March 30, 2026 17:27

sherwinski marked this pull request as ready for review March 30, 2026 20:01

claude Bot reviewed Mar 30, 2026

View reviewed changes

Comment thread src/shared/environment/detect.test.ts Outdated

Comment thread src/shared/environment/detect.ts

sherwinski added 13 commits March 31, 2026 11:22

test: add Safari push manual test

24e28fe

chore: remove unused eslint-disable

745dec5

chore: update package size limits

84ac235

style: sync formatting fixes

921d496

chore(devdeps): add @types/jest

f92dd8c

test(detect): import afterEach and beforeEach hooks

aafb684

chore: remove title from example index.html

3d4a57a

config(vite): optionally disable HTTPS

83c9469

chore: update pnpm-lock

12b5a32

sherwinski force-pushed the fix/safari-vapid-push-detection branch from 2da22a5 to 12b5a32 Compare March 31, 2026 15:24

Claude started work on behalf of sherwinski March 31, 2026 15:26 View session

Claude AI changed the title ~~Fix/safari vapid push detection~~ Addressing PR comments Mar 31, 2026

Claude finished work on behalf of sherwinski March 31, 2026 15:28

sherwinski changed the title ~~Addressing PR comments~~ fix: use legacy Safari push path only for existing legacy subscribers Mar 31, 2026

sherwinski requested review from abdulraqeeb33 and fadi-george March 31, 2026 15:53

fadi-george reviewed Mar 31, 2026

View reviewed changes

Comment thread src/shared/environment/detect.test.ts

fadi-george reviewed Mar 31, 2026

View reviewed changes

Comment thread src/shared/environment/detect.test.ts Outdated

fadi-george reviewed Mar 31, 2026

View reviewed changes

Comment thread vite.config.ts Outdated

style: run formatter on vite config file

cd3bb94

sherwinski added 3 commits March 31, 2026 16:41

test(detect): remove unneeded afterEach hooks

aa03266

style: run formatter on detect.test.ts

4d70d94

config: cleaner server https field eval

d0bfe7d

sherwinski requested a review from fadi-george March 31, 2026 20:46

fadi-george reviewed Mar 31, 2026

View reviewed changes

Comment thread tsconfig.json Outdated

chore(deps): remove @types/jest

05fed6a

sherwinski requested a review from fadi-george March 31, 2026 20:55

fadi-george reviewed Mar 31, 2026

View reviewed changes

fadi-george approved these changes Mar 31, 2026

View reviewed changes

sherwinski merged commit 6bfcc8b into main Mar 31, 2026
3 checks passed

sherwinski deleted the fix/safari-vapid-push-detection branch March 31, 2026 22:26

github-actions Bot mentioned this pull request Apr 1, 2026

Release 160603 #1451

Merged

sherwinski mentioned this pull request Apr 1, 2026

Have Safari use Standard Web Notifications #1329

Closed

16 tasks

		@@ -150,10 +150,10 @@ export default defineConfig({
		server: {

Conversation

sherwinski commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

1 Line Summary

Details

Systems Affected

Validation

Tests

Info

Checklist

Screenshots

Info

Safari 15.6 — new user, legacy path (BrowserStack)

Safari 26.2 — new user, VAPID path

Safari 26.2 — existing legacy subscriber preserved

Chrome 146 — no regression

Checklist

Uh oh!

Uh oh!

Uh oh!

sherwinski commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sherwinski commented Mar 31, 2026

Uh oh!

Claude AI commented Mar 31, 2026

Uh oh!

Uh oh!

fadi-george Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sherwinski Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fadi-george Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

sherwinski Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fadi-george Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fadi-george Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sherwinski Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

fadi-george commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fadi-george Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

abdulraqeeb33 commented Mar 31, 2026

Uh oh!

sherwinski commented Mar 31, 2026

Uh oh!

sherwinski commented Mar 31, 2026

Uh oh!

Uh oh!

fadi-george Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

sherwinski commented Mar 20, 2026 •

edited

Loading

sherwinski commented Mar 31, 2026 •

edited

Loading

fadi-george Mar 31, 2026 •

edited

Loading

sherwinski Mar 31, 2026 •

edited

Loading

fadi-george Mar 31, 2026 •

edited

Loading

fadi-george Mar 31, 2026 •

edited

Loading

fadi-george commented Mar 31, 2026 •

edited

Loading

fadi-george Mar 31, 2026 •

edited

Loading