From b724a625d0ee764b12fbe75787614fedb899208d Mon Sep 17 00:00:00 2001 From: Josh Kasten Date: Wed, 26 Jul 2023 16:50:00 +0000 Subject: [PATCH 1/4] remove no longer needed first page view check This check is no longer needed as the session manager and model repo handle these checks. The original motivation to remove this is to address a macOS Safari notification permission prompting issue where you can't await on I/O, otherwise the prompting is flagged not done from an end-user interaction. The isPushNotificationsEnabled() does disk I/O which we removed in this commit. --- src/shared/helpers/SubscriptionHelper.ts | 26 ++---------------------- 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/src/shared/helpers/SubscriptionHelper.ts b/src/shared/helpers/SubscriptionHelper.ts index 3f963b2c5..9ada21fe0 100755 --- a/src/shared/helpers/SubscriptionHelper.ts +++ b/src/shared/helpers/SubscriptionHelper.ts @@ -11,40 +11,18 @@ import Log from '../libraries/Log'; import { ContextSWInterface } from '../models/ContextSW'; import SdkEnvironment from '../managers/SdkEnvironment'; import { EnvironmentInfo } from '../../page/models/EnvironmentInfo'; -import { SessionOrigin } from '../models/Session'; import { Browser } from '../models/Browser'; -import { PushDeviceRecord } from '../models/PushDeviceRecord'; import { PermissionUtils } from '../utils/PermissionUtils'; -import MainHelper from './MainHelper'; export default class SubscriptionHelper { public static async registerForPush(): Promise { - const isPushEnabled = await OneSignal.context.subscriptionManager.isPushNotificationsEnabled(); - return await SubscriptionHelper.internalRegisterForPush(isPushEnabled); + return await SubscriptionHelper.internalRegisterForPush(); } - public static async internalRegisterForPush(isPushEnabled: boolean): Promise { + public static async internalRegisterForPush(): Promise { const context: ContextSWInterface = OneSignal.context; let subscription: Subscription | null = null; - /* - Within the same page navigation (the same session), do not register for - push if the user is already subscribed, otherwise the user will have its - session count incremented on each page refresh. However, if the user is - not subscribed, subscribe. - */ - if (isPushEnabled && !context.pageViewManager.isFirstPageView()) { - Log.debug('Not registering for push because the user is subscribed and this is not the first page view.'); - Log.debug("But we want to rekindle their session."); - const deviceId = await MainHelper.getDeviceId(); - if (deviceId) { - const deviceRecord: PushDeviceRecord = await MainHelper.createDeviceRecord(OneSignal.config.appId, true); - await OneSignal.context.sessionManager.upsertSession(deviceId, deviceRecord, SessionOrigin.PageRefresh); - } else { - Log.error("Should have been impossible to have push as enabled but no device id."); - } - return null; - } switch (SdkEnvironment.getWindowEnv()) { case WindowEnvironmentKind.Host: From 04c97aed1731552a7e7a12dc73f97c73179bc1f8 Mon Sep 17 00:00:00 2001 From: Josh Kasten Date: Wed, 26 Jul 2023 17:01:20 +0000 Subject: [PATCH 2/4] remove unnecessary optedOut check Since we no longer support the os.tc workaround there is no need to check the the indexDb for user preferences. Native notification permission status is checked later making this unnecessary as well from normal HTTPS setups. --- src/shared/helpers/InitHelper.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/shared/helpers/InitHelper.ts b/src/shared/helpers/InitHelper.ts index b3716535b..8c5bbbb7c 100755 --- a/src/shared/helpers/InitHelper.ts +++ b/src/shared/helpers/InitHelper.ts @@ -118,6 +118,8 @@ export default class InitHelper { await OneSignalEvent.trigger(OneSignal.EVENTS.SDK_INITIALIZED); } + // In-Addition to registering for push, this shows a native browser + // notification permission prompt, if needed. public static async registerForPushNotifications(options: RegisterOptions = {}): Promise { if (options && options.modalPrompt) { /* Show the HTTPS fullscreen modal permission message. */ @@ -146,15 +148,7 @@ export default class InitHelper { return; } - /* - * We don't want to resubscribe if the user is opted out, and we can't check on HTTP, because the promise will - * prevent the popup from opening. - */ - const subscription = await Database.getSubscription(); - const isOptedOut = subscription.optedOut; - if (!isOptedOut) { - await SubscriptionHelper.registerForPush(); - } + await SubscriptionHelper.registerForPush(); } /** From 03d59892ee6c098052eef2252ddd0a6dc489af5b Mon Sep 17 00:00:00 2001 From: Josh Kasten Date: Wed, 26 Jul 2023 18:09:07 +0000 Subject: [PATCH 3/4] fix safari legacy push token detection Corrected missing logic changes that PR #1002 introduced in main but were not accounted for in the last rebase. Tested on masOS 13.3.1 with Safari 16.4 to confirm the browser subscribes to push correctly with the backend. --- src/page/userModel/FuturePushSubscriptionRecord.ts | 14 +++++++------- src/shared/helpers/MainHelper.ts | 5 ++--- src/shared/helpers/SubscriptionHelper.ts | 8 ++++---- src/sw/serviceWorker/ServiceWorker.ts | 12 ++---------- 4 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/page/userModel/FuturePushSubscriptionRecord.ts b/src/page/userModel/FuturePushSubscriptionRecord.ts index f75b6686b..c1e43bcb9 100644 --- a/src/page/userModel/FuturePushSubscriptionRecord.ts +++ b/src/page/userModel/FuturePushSubscriptionRecord.ts @@ -4,6 +4,7 @@ import { EnvironmentInfoHelper } from "../helpers/EnvironmentInfoHelper"; import { RawPushSubscription } from "src/shared/models/RawPushSubscription"; import OneSignalUtils from "../../shared/utils/OneSignalUtils"; import { SubscriptionStateKind } from "../../shared/models/SubscriptionStateKind"; +import Environment from "../../shared/helpers/Environment"; export default class FuturePushSubscriptionRecord implements Serializable { readonly type: SubscriptionType; @@ -33,10 +34,10 @@ export default class FuturePushSubscriptionRecord implements Serializable { } private _getToken(subscription: RawPushSubscription): string | undefined { - const browser = OneSignalUtils.redetectBrowserUserAgent(); - const isLegacySafari = browser.safari && browser.version < 16; - return isLegacySafari ? subscription.safariDeviceToken : subscription.w3cEndpoint ? - subscription.w3cEndpoint.toString() : undefined; + if (subscription.w3cEndpoint) { + return subscription.w3cEndpoint.toString(); + } + return subscription.safariDeviceToken; } serialize(): FutureSubscriptionModel { @@ -60,11 +61,10 @@ export default class FuturePushSubscriptionRecord implements Serializable { if (browser.firefox) { return SubscriptionType.FirefoxPush; } - // TO DO: update to use feature detection - if (browser.safari && browser.version >= 16) { + if (Environment.useSafariVapidPush()) { return SubscriptionType.SafariPush; } - if (browser.safari && browser.version < 16) { + if (Environment.useSafariLegacyPush()) { return SubscriptionType.SafariLegacyPush; } if (browser.msedge) { diff --git a/src/shared/helpers/MainHelper.ts b/src/shared/helpers/MainHelper.ts index f8aa830ec..c132e291d 100755 --- a/src/shared/helpers/MainHelper.ts +++ b/src/shared/helpers/MainHelper.ts @@ -12,7 +12,7 @@ import Database from '../services/Database'; import OneSignalUtils from '../utils/OneSignalUtils'; import { PermissionUtils } from '../utils/PermissionUtils'; import OneSignalEvent from '../services/OneSignalEvent'; -import { supportsVapidPush } from '../../page/utils/BrowserSupportsPush'; +import Environment from './Environment'; export default class MainHelper { @@ -219,8 +219,7 @@ export default class MainHelper { // TO DO: unit test static async getCurrentPushToken(): Promise { - // legacy safari (<16) - if (window.safari && !supportsVapidPush()) { + if (Environment.useSafariLegacyPush()) { const safariToken = window.safari?.pushNotification?.permission(OneSignal.config.safariWebId).deviceToken; return safariToken?.toLowerCase() || undefined; } diff --git a/src/shared/helpers/SubscriptionHelper.ts b/src/shared/helpers/SubscriptionHelper.ts index 9ada21fe0..8ac51f0b6 100755 --- a/src/shared/helpers/SubscriptionHelper.ts +++ b/src/shared/helpers/SubscriptionHelper.ts @@ -11,8 +11,8 @@ import Log from '../libraries/Log'; import { ContextSWInterface } from '../models/ContextSW'; import SdkEnvironment from '../managers/SdkEnvironment'; import { EnvironmentInfo } from '../../page/models/EnvironmentInfo'; -import { Browser } from '../models/Browser'; import { PermissionUtils } from '../utils/PermissionUtils'; +import Environment from './Environment'; export default class SubscriptionHelper { public static async registerForPush(): Promise { @@ -117,7 +117,7 @@ export default class SubscriptionHelper { return subscription; } - static getRawPushSubscriptionForSafari(safariWebId: string): RawPushSubscription { + static getRawPushSubscriptionForLegacySafari(safariWebId: string): RawPushSubscription { const subscription = new RawPushSubscription(); const { deviceToken: existingDeviceToken } = window.safari.pushNotification.permission(safariWebId); @@ -145,8 +145,8 @@ export default class SubscriptionHelper { static async getRawPushSubscription( environmentInfo: EnvironmentInfo, safariWebId: string ):Promise { - if (environmentInfo.browserType === Browser.Safari) { - return SubscriptionHelper.getRawPushSubscriptionForSafari(safariWebId); + if (Environment.useSafariLegacyPush()) { + return SubscriptionHelper.getRawPushSubscriptionForLegacySafari(safariWebId); } if (environmentInfo.isUsingSubscriptionWorkaround) { diff --git a/src/sw/serviceWorker/ServiceWorker.ts b/src/sw/serviceWorker/ServiceWorker.ts index a0e08ef84..5ca80447b 100755 --- a/src/sw/serviceWorker/ServiceWorker.ts +++ b/src/sw/serviceWorker/ServiceWorker.ts @@ -1,5 +1,3 @@ -import bowser from "bowser"; - import Environment from "../../shared/helpers/Environment"; import ContextSW from "../../shared/models/ContextSW"; import Database from "../../shared/services/Database"; @@ -33,6 +31,7 @@ import { RawPushSubscription } from "../../../src/shared/models/RawPushSubscript import { DeliveryPlatformKind } from "../../shared/models/DeliveryPlatformKind"; import { NotificationClickEvent } from "../../page/models/NotificationEvent"; import { RawNotificationPayload } from "../../shared/models/RawNotificationPayload"; +import { bowserCastle } from "../../shared/utils/bowserCastle"; declare const self: ServiceWorkerGlobalScope & OSServiceWorkerFields; @@ -75,13 +74,6 @@ export class ServiceWorker { return Database; } - /** - * Describes the current browser name and version. - */ - static get browser() { - return bowser; - } - /** * Allows message passing between this service worker and pages on the same domain. * Clients include any HTTPS site page, or the nested iFrame pointing to OneSignal on any HTTP site. This allows @@ -392,7 +384,7 @@ export class ServiceWorker { * to be safe we are disabling it for all Safari browsers. */ static browserSupportsConfirmedDelivery(): boolean { - return !bowser.safari + return bowserCastle().name !== 'safari' } /** From 6ede0327d023460f34d5c6c364d05c8163e71e5b Mon Sep 17 00:00:00 2001 From: Josh Kasten Date: Wed, 26 Jul 2023 18:17:48 +0000 Subject: [PATCH 4/4] updated Safari 16 comments --- src/shared/helpers/InitHelper.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/shared/helpers/InitHelper.ts b/src/shared/helpers/InitHelper.ts index 8c5bbbb7c..8166de26d 100755 --- a/src/shared/helpers/InitHelper.ts +++ b/src/shared/helpers/InitHelper.ts @@ -364,14 +364,15 @@ export default class InitHelper { try { if (navigator.permissions) { const permissionStatus = await navigator.permissions.query({ name: 'notifications' }); + // NOTE: Safari 16.4 has a bug where onchange callback never fires permissionStatus.onchange = function() { triggerNotificationPermissionChanged(); }; } } catch (e) { - // navigator.permissions.query({ name: 'notifications' }) currently not supported on Safari 16 (beta) - // as of June 2022 - Log.warn(`Could not install native prompt permission change hook w/ error: ${e}`); + // Some browsers (Safari 16.3 and older) have the API navigator.permissions.query, but don't support the + // { name: 'notifications' } param and throws. + Log.warn(`Could not install native notification permission change hook w/ error: ${e}`); } }