Remove subdomain code (os.tc) #1142

jkasten2 · 2023-11-23T04:02:03Z

Description

1 Line Summary

Remove code for the OneSignal Subdomain label (os.tc) as supported was dropped in v16.

Details

Motivation

Motivation to removing this deadcode is to make upcoming changes easier to make. This PR also results in a noticeable size reduction in the SDK.

Scope

This PR addresses deleting anything related to the dropped subdomain / os.tc feature. Very minor refactoring was done to clean up some code paths related to the feature removal.

Validation

Test on Windows 11 23H2 with Chrome 119.

addTag
Subscribing for notifications
Clicking a notification
Confirmed deliveries
Session count and time
Slidedown
Bell
Custom subscribe link

Tested on macOS 13.6.1 Safari 17.1

Subscribing for notifications
Clicking a notification
Slidedown
Bell
Custom subscribe link

Tested on iOS 16.7.2

Subscribing
Clicking a notification
Slidedown
Bell
Custom subscribe link

Tests

Info

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

Checklist

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

Related Tickets

This change is

IsUsingSubscriptionWorkaround is going to be removed in future commits so moved this into a more permanent place, in the app config logic.

Removed the internalIsUsingSubscriptionWorkaround function and all code dependent on it.

Removed isUsingSubscriptionWorkaround() and all usages of it

Remove SdkEnvironment.getIntegration() as the SDK only supports IntegrationKind.Secure now. If the developer attempted to init the SDK in a non-secure env then we would have thrown before this.

Remove AltOriginManager as it was only used to manage proxy iframes to os.tc.

Remove canTalkToServiceWorker() as we already check HTTPS on init.

Remove DeprecatedApiError, this could have been removed with the v16 release since these APIs were droped then

Removed all the unsupported HTTP subdmain (os.tc) types from WindowEnvironmentKind and code using them.

Remove all notification permission logic for HTTP / subdomain detection

This is no longer need and corrected related http checks to use window.isSecureContext instead. (as localhost is a secure context)

This is always HTTPS now as HTTP is no longer supported.

OneSignal must be included on the intented page and we no longer support the os.tc / onesginal.com subdomain option.

Made this change to make this more clear that this is an unsupported setting. Cleaned up some usages of subdomain as well.

This was related to subdomain which was removed in the previous commit

useSafariLegacyPush is already checked in getSubscriptionStateForSecure

This was used to talk between the removed os.tc iframe and the main page

OneSignal SDK can't be run in an iframe so this detection can be removed now.

It isn't clear why this started failing recently, but tests no longer fail after this change.

rgomezp · 2023-12-11T22:35:59Z

Other than what we discussed, LGTM.

One nit is the tests are logging an error (tests still pass) -- this may or may not be something that predates these changes

    Error: OneSignal service worker not found!
        at SubscriptionManager.subscribeFcmFromPage (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/managers/SubscriptionManager.ts:513:13)
        at SubscriptionManager.subscribe (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/managers/SubscriptionManager.ts:162:13)
        at Function.internalRegisterForPush (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/SubscriptionHelper.ts:29:35)
        at Function.registerForPush (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/SubscriptionHelper.ts:19:12)
        at Function.registerForPushNotifications (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/InitHelper.ts:104:5)
        at PromptsManager.internalShowNativePrompt (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/page/managers/PromptsManager.ts:189:5)
        at NotificationsNamespace.requestPermission (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/onesignal/NotificationsNamespace.ts:142:5)
        at PushSubscriptionNamespace.optIn (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/onesignal/PushSubscriptionNamespace.ts:96:7)
        at Object.<anonymous> (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/__test__/unit/push/registerForPush.test.ts:31:5)

      38 |           await EventHelper.checkAndTriggerSubscriptionChanged();
      39 |         } catch (e) {
    > 40 |           Log.error(e);
         |               ^
      41 |         }
      42 |         break;
      43 |       default:

      at Function.error [as internalRegisterForPush] (src/shared/helpers/SubscriptionHelper.ts:40:15)
      at Function.registerForPush (src/shared/helpers/SubscriptionHelper.ts:19:12)
      at Function.registerForPushNotifications (src/shared/helpers/InitHelper.ts:104:5)
      at PromptsManager.internalShowNativePrompt (src/page/managers/PromptsManager.ts:[189](https://github.com/OneSignal/OneSignal-Website-SDK/actions/runs/7024669946/job/19113899601?pr=1142#step:6:190):5)
      at NotificationsNamespace.requestPermission (src/onesignal/NotificationsNamespace.ts:142:5)
      at PushSubscriptionNamespace.optIn (src/onesignal/PushSubscriptionNamespace.ts:96:7)
      at Object.<anonymous> (__test__/unit/push/registerForPush.test.ts:31:5)

  console.error
    Error: OneSignal service worker not found!
        at SubscriptionManager.subscribeFcmFromPage (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/managers/SubscriptionManager.ts:513:13)
        at SubscriptionManager.subscribe (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/managers/SubscriptionManager.ts:162:13)
        at Function.internalRegisterForPush (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/SubscriptionHelper.ts:29:35)
        at Function.registerForPush (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/SubscriptionHelper.ts:19:12)
        at Function.registerForPushNotifications (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/InitHelper.ts:104:5)
        at Object.<anonymous> (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/__test__/unit/push/registerForPush.test.ts:41:5)

      38 |           await EventHelper.checkAndTriggerSubscriptionChanged();
      39 |         } catch (e) {
    > 40 |           Log.error(e);
         |               ^
      41 |         }
      42 |         break;
      43 |       default:

      at Function.error [as internalRegisterForPush] (src/shared/helpers/SubscriptionHelper.ts:40:15)
      at Function.registerForPush (src/shared/helpers/SubscriptionHelper.ts:19:12)
      at Function.registerForPushNotifications (src/shared/helpers/InitHelper.ts:104:5)
      at Object.<anonymous> (__test__/unit/push/registerForPush.test.ts:41:5)
      ```

jkasten2 force-pushed the refactor/rm_ostc_code branch 2 times, most recently from 338036a to 39ed293 Compare November 23, 2023 05:23

jkasten2 requested review from rgomezp and emawby November 27, 2023 18:51

jkasten2 changed the title ~~[WIP] [Refactor] Remove subdomain code~~ [WIP] Remove subdomain code (os.tc) Nov 27, 2023

jkasten2 added 25 commits November 28, 2023 16:19

move unsupported os.tc check

ef3ec2c

IsUsingSubscriptionWorkaround is going to be removed in future commits so moved this into a more permanent place, in the app config logic.

rm internalIsUsingSubscriptionWorkaround

090d872

Removed the internalIsUsingSubscriptionWorkaround function and all code dependent on it.

rm isUsingSubscriptionWorkaround()

4c06923

Removed isUsingSubscriptionWorkaround() and all usages of it

rm isUsingSubscriptionWorkaround

3c53766

rm getIntegration, everything is secure now

ab3c9a1

Remove SdkEnvironment.getIntegration() as the SDK only supports IntegrationKind.Secure now. If the developer attempted to init the SDK in a non-secure env then we would have thrown before this.

rm AltOriginManager

42848b9

Remove AltOriginManager as it was only used to manage proxy iframes to os.tc.

rm canTalkToServiceWorker()

eb2176b

Remove canTalkToServiceWorker() as we already check HTTPS on init.

rm ONESIGNAL_HTTP_PROMPT_SHOWN

a15799b

rm getOneSignalSubdomainNotificationPermission

a2ad30e

rm DeprecatedApiError

098cd76

Remove DeprecatedApiError, this could have been removed with the v16 release since these APIs were droped then

rm LegacyManager and metrics for dropped APIs

efcfd85

rm httpPermissionRequest

ecf8389

Clean up WindowEnvironmentKind

7a15e3d

Removed all the unsupported HTTP subdmain (os.tc) types from WindowEnvironmentKind and code using them.

rm lastKnownHostUrl and logic around it

fa4da7b

remove all proxy frame classes

9f42457

rm HTTP logic from PermisionManager

1d1c652

Remove all notification permission logic for HTTP / subdomain detection

rm isFrameContextInsecure

3cc2b05

This is no longer need and corrected related http checks to use window.isSecureContext instead. (as localhost is a secure context)

rm MissingSubdomain

964dbe7

rm isHttps checks, this is already true now

23ba387

rm HttpHttpsEnvironment from tests

d4211ea

This is always HTTPS now as HTTP is no longer supported.

rm subdomain checks from checkRestrictedOrigin

541d93d

OneSignal must be included on the intented page and we no longer support the os.tc / onesginal.com subdomain option.

AppConfig.subdomain to hasUnsupportedSubdomain

cf614a6

Made this change to make this more clear that this is an unsupported setting. Cleaned up some usages of subdomain as well.

rm httpUseOneSignalCom

3a4efe7

This was related to subdomain which was removed in the previous commit

remove reduntent useSafariLegacyPush check

443c8b8

useSafariLegacyPush is already checked in getSubscriptionStateForSecure

corrected ForSecure naming

f8ec723

jkasten2 added 10 commits November 28, 2023 16:19

rm isInsecureOrigin(), as it is deadcode

522cf9c

rm postmam

12e835a

This was used to talk between the removed os.tc iframe and the main page

clean up comments referring to subdomain feature

5dc8959

rm misc types and properites related to os.tc

8a54932

remove unused imports

0235268

clean up Safari in iframe detection

97fdfd1

OneSignal SDK can't be run in an iframe so this detection can be removed now.

remove onSession HTTP test

e1be1bb

rm dead code getSubscriptionStateForInsecure

593f80f

rm os.tc from cert gen config

6efdb70

fix flaky test carry over

d869a8e

It isn't clear why this started failing recently, but tests no longer fail after this change.

jkasten2 force-pushed the refactor/rm_ostc_code branch from 0f7a5ad to d869a8e Compare November 28, 2023 21:19

jkasten2 changed the title ~~[WIP] Remove subdomain code (os.tc)~~ Remove subdomain code (os.tc) Dec 6, 2023

rgomezp approved these changes Dec 11, 2023

View reviewed changes

jkasten2 merged commit b7121cd into main Dec 13, 2023
4 checks passed

jkasten2 deleted the refactor/rm_ostc_code branch December 13, 2023 00:37

jkasten2 mentioned this pull request Dec 13, 2023

160101 Release #1148

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove subdomain code (os.tc) #1142

Remove subdomain code (os.tc) #1142

jkasten2 commented Nov 23, 2023 •

edited

Loading

rgomezp commented Dec 11, 2023

Remove subdomain code (os.tc) #1142

Remove subdomain code (os.tc) #1142

Conversation

jkasten2 commented Nov 23, 2023 • edited Loading

Description

1 Line Summary

Details

Motivation

Scope

Validation

Tests

Info

Checklist

Screenshots

Info

Checklist

Related Tickets

rgomezp commented Dec 11, 2023

jkasten2 commented Nov 23, 2023 •

edited

Loading