From d9db73ea056e2e6e2bde8e0c1de4a7090984ae9a Mon Sep 17 00:00:00 2001 From: Aman Harwara Date: Tue, 25 Oct 2022 21:38:29 +0530 Subject: [PATCH] fix: on mobile open links from editor in external browser (#1860) --- .../WebFrame/DeviceInterface.template.js | 2 +- packages/mobile/src/Lib/Interface.ts | 14 ++++++ packages/mobile/src/MobileWebAppContainer.tsx | 33 ++++++++++++- .../models/src/Domain/Device/Environment.ts | 1 - .../Domain/Device/MobileDeviceInterface.ts | 3 ++ .../snjs/lib/Application/Application.spec.ts | 4 ++ packages/snjs/lib/Application/Application.ts | 17 ++----- .../Application/Options/OptionalOptions.ts | 8 --- packages/snjs/lib/Application/Platforms.ts | 13 +---- .../ComponentManager/ComponentManager.spec.ts | 11 ++++- .../ComponentManager/ComponentManager.ts | 49 +++++++++++-------- .../Services/Storage/DiskStorageService.ts | 2 +- .../NativeMobileWeb/MobileWebReceiver.ts | 2 +- .../javascripts/Utils/ManageSubscription.ts | 2 +- 14 files changed, 99 insertions(+), 62 deletions(-) diff --git a/packages/mobile/WebFrame/DeviceInterface.template.js b/packages/mobile/WebFrame/DeviceInterface.template.js index 0e7429ed3..672e12300 100644 --- a/packages/mobile/WebFrame/DeviceInterface.template.js +++ b/packages/mobile/WebFrame/DeviceInterface.template.js @@ -2,7 +2,7 @@ class WebProcessDeviceInterface { constructor(messageSender) { this.appVersion = '1.2.3' - this.environment = 4 + this.environment = 3 this.databases = [] this.messageSender = messageSender } diff --git a/packages/mobile/src/Lib/Interface.ts b/packages/mobile/src/Lib/Interface.ts index 4cd694039..7dd5d49ba 100644 --- a/packages/mobile/src/Lib/Interface.ts +++ b/packages/mobile/src/Lib/Interface.ts @@ -11,6 +11,7 @@ import { RawKeychainValue, removeFromArray, TransferPayload, + UuidString, } from '@standardnotes/snjs' import { Alert, Linking, PermissionsAndroid, Platform, StatusBar } from 'react-native' import FileViewer from 'react-native-file-viewer' @@ -79,6 +80,7 @@ export class MobileDevice implements MobileDeviceInterface { private eventObservers: MobileDeviceEventHandler[] = [] public isDarkMode = false public statusBarBgColor: string | undefined + private componentUrls: Map = new Map() constructor( private stateObserverService?: AppStateObserverService, @@ -596,4 +598,16 @@ export class MobileDevice implements MobileDeviceInterface { }, ) } + + addComponentUrl(componentUuid: UuidString, componentUrl: string) { + this.componentUrls.set(componentUuid, componentUrl) + } + + removeComponentUrl(componentUuid: UuidString) { + this.componentUrls.delete(componentUuid) + } + + isUrlComponentUrl(url: string): boolean { + return Array.from(this.componentUrls.values()).includes(url) + } } diff --git a/packages/mobile/src/MobileWebAppContainer.tsx b/packages/mobile/src/MobileWebAppContainer.tsx index d7c5a9e25..160fc76bb 100644 --- a/packages/mobile/src/MobileWebAppContainer.tsx +++ b/packages/mobile/src/MobileWebAppContainer.tsx @@ -3,6 +3,7 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Keyboard, Platform } from 'react-native' import VersionInfo from 'react-native-version-info' import { WebView, WebViewMessageEvent } from 'react-native-webview' +import { OnShouldStartLoadWithRequest } from 'react-native-webview/lib/WebViewTypes' import pjson from '../package.json' import { AndroidBackHandlerService } from './AndroidBackHandlerService' import { AppStateObserverService } from './AppStateObserverService' @@ -23,6 +24,7 @@ export const MobileWebAppContainer = () => { const MobileWebAppContents = ({ destroyAndReload }: { destroyAndReload: () => void }) => { const webViewRef = useRef(null) + const sourceUri = (Platform.OS === 'android' ? 'file:///android_asset/' : '') + 'Web.bundle/src/index.html' const stateService = useMemo(() => new AppStateObserverService(), []) const androidBackHandlerService = useMemo(() => new AndroidBackHandlerService(), []) @@ -99,7 +101,7 @@ const MobileWebAppContents = ({ destroyAndReload }: { destroyAndReload: () => vo class WebProcessDeviceInterface { constructor(messageSender) { this.appVersion = '${pjson.version} (${VersionInfo.buildVersion})' - this.environment = 4 + this.environment = 3 this.platform = ${device.platform} this.databases = [] this.messageSender = messageSender @@ -195,6 +197,34 @@ const MobileWebAppContents = ({ destroyAndReload }: { destroyAndReload: () => vo webViewRef.current?.postMessage(JSON.stringify({ messageId, returnValue, messageType: 'reply' })) } + const onShouldStartLoadWithRequest: OnShouldStartLoadWithRequest = (request) => { + /** + * We want to handle link clicks within an editor by opening the browser + * instead of loading inline. On iOS, onShouldStartLoadWithRequest is + * called for all requests including the initial request to load the editor. + * On iOS, clicks in the editors have a navigationType of 'click', but on + * Android, this is not the case (no navigationType). + * However, on Android, this function is not called for the initial request. + * So that might be one way to determine if this request is a click or the + * actual editor load request. But I don't think it's safe to rely on this + * being the case in the future. So on Android, we'll handle url loads only + * if the url isn't equal to the editor url. + */ + + const shouldStopRequest = + (Platform.OS === 'ios' && request.navigationType === 'click') || + (Platform.OS === 'android' && request.url !== sourceUri) + + const isComponentUrl = device.isUrlComponentUrl(request.url) + + if (shouldStopRequest && !isComponentUrl) { + device.openUrl(request.url) + return false + } + + return true + } + return ( vo onRenderProcessGone={() => { webViewRef.current?.reload() }} + onShouldStartLoadWithRequest={onShouldStartLoadWithRequest} allowFileAccess={true} allowUniversalAccessFromFileURLs={true} injectedJavaScriptBeforeContentLoaded={injectedJS} diff --git a/packages/models/src/Domain/Device/Environment.ts b/packages/models/src/Domain/Device/Environment.ts index b0fd2a8c7..391584c78 100644 --- a/packages/models/src/Domain/Device/Environment.ts +++ b/packages/models/src/Domain/Device/Environment.ts @@ -2,5 +2,4 @@ export enum Environment { Web = 1, Desktop = 2, Mobile = 3, - NativeMobileWeb = 4, } diff --git a/packages/services/src/Domain/Device/MobileDeviceInterface.ts b/packages/services/src/Domain/Device/MobileDeviceInterface.ts index 9063b04af..aa228b6ac 100644 --- a/packages/services/src/Domain/Device/MobileDeviceInterface.ts +++ b/packages/services/src/Domain/Device/MobileDeviceInterface.ts @@ -17,4 +17,7 @@ export interface MobileDeviceInterface extends DeviceInterface { downloadBase64AsFile(base64: string, filename: string, saveInTempLocation?: boolean): Promise previewFile(base64: string, filename: string): Promise exitApp(confirm?: boolean): void + addComponentUrl(componentUuid: string, componentUrl: string): void + removeComponentUrl(componentUuid: string): void + isUrlComponentUrl(url: string): boolean } diff --git a/packages/snjs/lib/Application/Application.spec.ts b/packages/snjs/lib/Application/Application.spec.ts index d9a18be9e..8381b2c0f 100644 --- a/packages/snjs/lib/Application/Application.spec.ts +++ b/packages/snjs/lib/Application/Application.spec.ts @@ -1,3 +1,7 @@ +/** + * @jest-environment jsdom + */ + import { SNLog } from './../Log' import { PureCryptoInterface } from '@standardnotes/sncrypto-common' import { AlertService, DeviceInterface, namespacedKey, RawStorageKey } from '@standardnotes/services' diff --git a/packages/snjs/lib/Application/Application.ts b/packages/snjs/lib/Application/Application.ts index 99e89d454..0032fcb88 100644 --- a/packages/snjs/lib/Application/Application.ts +++ b/packages/snjs/lib/Application/Application.ts @@ -940,7 +940,7 @@ export class SNApplication implements ApplicationInterface, AppGroupManagedAppli } isNativeMobileWeb() { - return this.environment === Environment.NativeMobileWeb + return this.environment === Environment.Mobile } getDeinitMode(): DeinitMode { @@ -1353,10 +1353,7 @@ export class SNApplication implements ApplicationInterface, AppGroupManagedAppli } private createComponentManager() { - const MaybeSwappedComponentManager = this.getClass( - InternalServices.SNComponentManager, - ) - this.componentManagerService = new MaybeSwappedComponentManager( + this.componentManagerService = new InternalServices.SNComponentManager( this.itemManager, this.syncService, this.featuresService, @@ -1365,6 +1362,7 @@ export class SNApplication implements ApplicationInterface, AppGroupManagedAppli this.environment, this.platform, this.internalEventBus, + this.deviceInterface, ) this.services.push(this.componentManagerService) } @@ -1647,13 +1645,4 @@ export class SNApplication implements ApplicationInterface, AppGroupManagedAppli this.statusService = new ExternalServices.StatusService(this.internalEventBus) this.services.push(this.statusService) } - - private getClass(base: T) { - const swapClass = this.options.swapClasses?.find((candidate) => candidate.swap === base) - if (swapClass) { - return swapClass.with as T - } else { - return base - } - } } diff --git a/packages/snjs/lib/Application/Options/OptionalOptions.ts b/packages/snjs/lib/Application/Options/OptionalOptions.ts index b1d1df9a8..af314a29d 100644 --- a/packages/snjs/lib/Application/Options/OptionalOptions.ts +++ b/packages/snjs/lib/Application/Options/OptionalOptions.ts @@ -10,14 +10,6 @@ export interface ApplicationDisplayOptions { } export interface ApplicationOptionalConfiguratioOptions { - /** - * Gives consumers the ability to provide their own custom - * subclass for a service. swapClasses should be an array of key/value pairs - * consisting of keys 'swap' and 'with'. 'swap' is the base class you wish to replace, - * and 'with' is the custom subclass to use. - */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - swapClasses?: { swap: any; with: any }[] /** * URL for WebSocket providing permissions and roles information. */ diff --git a/packages/snjs/lib/Application/Platforms.ts b/packages/snjs/lib/Application/Platforms.ts index 7476bcdf0..d0a0b89bc 100644 --- a/packages/snjs/lib/Application/Platforms.ts +++ b/packages/snjs/lib/Application/Platforms.ts @@ -28,22 +28,11 @@ export function platformToString(platform: Platform) { return map[platform] } -export function environmentFromString(string: string) { - const map: Record = { - web: Environment.Web, - desktop: Environment.Desktop, - mobile: Environment.Mobile, - nativeMobileWeb: Environment.NativeMobileWeb, - } - return map[string] -} - export function environmentToString(environment: Environment) { const map = { [Environment.Web]: 'web', [Environment.Desktop]: 'desktop', - [Environment.Mobile]: 'mobile', - [Environment.NativeMobileWeb]: 'native-mobile-web', + [Environment.Mobile]: 'native-mobile-web', } return map[environment] } diff --git a/packages/snjs/lib/Services/ComponentManager/ComponentManager.spec.ts b/packages/snjs/lib/Services/ComponentManager/ComponentManager.spec.ts index 881d58b04..2c3e265c8 100644 --- a/packages/snjs/lib/Services/ComponentManager/ComponentManager.spec.ts +++ b/packages/snjs/lib/Services/ComponentManager/ComponentManager.spec.ts @@ -14,7 +14,12 @@ import { } from '@standardnotes/features' import { ContentType } from '@standardnotes/common' import { GenericItem, SNComponent, Environment, Platform } from '@standardnotes/models' -import { DesktopManagerInterface, InternalEventBusInterface, AlertService } from '@standardnotes/services' +import { + DesktopManagerInterface, + InternalEventBusInterface, + AlertService, + DeviceInterface, +} from '@standardnotes/services' import { ItemManager } from '@Lib/Services/Items/ItemManager' import { SNFeaturesService } from '@Lib/Services/Features/FeaturesService' import { SNComponentManager } from './ComponentManager' @@ -27,6 +32,7 @@ describe('featuresService', () => { let syncService: SNSyncService let prefsService: SNPreferencesService let internalEventBus: InternalEventBusInterface + let device: DeviceInterface const desktopExtHost = 'http://localhost:123' @@ -53,6 +59,7 @@ describe('featuresService', () => { environment, platform, internalEventBus, + device, ) manager.setDesktopManager(desktopManager) return manager @@ -81,6 +88,8 @@ describe('featuresService', () => { internalEventBus = {} as jest.Mocked internalEventBus.publish = jest.fn() + + device = {} as jest.Mocked }) const nativeComponent = (identifier?: FeatureIdentifier, file_type?: FeatureDescription['file_type']) => { diff --git a/packages/snjs/lib/Services/ComponentManager/ComponentManager.ts b/packages/snjs/lib/Services/ComponentManager/ComponentManager.ts index a99e86507..b3c9ac622 100644 --- a/packages/snjs/lib/Services/ComponentManager/ComponentManager.ts +++ b/packages/snjs/lib/Services/ComponentManager/ComponentManager.ts @@ -36,6 +36,8 @@ import { DesktopManagerInterface, InternalEventBusInterface, AlertService, + DeviceInterface, + isMobileDevice, } from '@standardnotes/services' const DESKTOP_URL_PREFIX = 'sn://' @@ -82,23 +84,21 @@ export class SNComponentManager private environment: Environment, private platform: Platform, protected override internalEventBus: InternalEventBusInterface, + private device: DeviceInterface, ) { super(internalEventBus) this.loggingEnabled = false this.addItemObserver() - /* On mobile, events listeners are handled by a respective component */ - if (environment !== Environment.Mobile) { - window.addEventListener - ? window.addEventListener('focus', this.detectFocusChange, true) - : window.attachEvent('onfocusout', this.detectFocusChange) - window.addEventListener - ? window.addEventListener('blur', this.detectFocusChange, true) - : window.attachEvent('onblur', this.detectFocusChange) + window.addEventListener + ? window.addEventListener('focus', this.detectFocusChange, true) + : window.attachEvent('onfocusout', this.detectFocusChange) + window.addEventListener + ? window.addEventListener('blur', this.detectFocusChange, true) + : window.attachEvent('onblur', this.detectFocusChange) - window.addEventListener('message', this.onWindowMessage, true) - } + window.addEventListener('message', this.onWindowMessage, true) } get isDesktop(): boolean { @@ -143,7 +143,7 @@ export class SNComponentManager this.removeItemObserver?.() ;(this.removeItemObserver as unknown) = undefined - if (window && !this.isMobile) { + if (window) { window.removeEventListener('focus', this.detectFocusChange, true) window.removeEventListener('blur', this.detectFocusChange, true) window.removeEventListener('message', this.onWindowMessage, true) @@ -221,9 +221,23 @@ export class SNComponentManager addItemObserver(): void { this.removeItemObserver = this.itemManager.addObserver( [ContentType.Component, ContentType.Theme], - ({ changed, inserted, source }) => { + ({ changed, inserted, removed, source }) => { const items = [...changed, ...inserted] this.handleChangedComponents(items, source) + + const device = this.device + if (isMobileDevice(device) && 'addComponentUrl' in device) { + inserted.forEach((component) => { + const url = this.urlForComponent(component) + if (url) { + device.addComponentUrl(component.uuid, url) + } + }) + + removed.forEach((component) => { + device.removeComponentUrl(component.uuid) + }) + } }, ) } @@ -271,9 +285,6 @@ export class SNComponentManager } getActiveThemes(): SNTheme[] { - if (this.environment === Environment.Mobile) { - throw Error('getActiveThemes must be handled separately by mobile') - } return this.componentsForArea(ComponentArea.Themes).filter((theme) => { return theme.active }) as SNTheme[] @@ -301,14 +312,10 @@ export class SNComponentManager } } - const isWeb = this.environment === Environment.Web - const isMobileWebView = this.environment === Environment.NativeMobileWeb + const isMobile = this.environment === Environment.Mobile if (nativeFeature) { - if (!isWeb && !isMobileWebView) { - throw Error('Mobile must override urlForComponent to handle native paths') - } let baseUrlRequiredForThemesInsideEditors = window.location.origin - if (isMobileWebView) { + if (isMobile) { baseUrlRequiredForThemesInsideEditors = window.location.href.split('/index.html')[0] } return `${baseUrlRequiredForThemesInsideEditors}/components/assets/${component.identifier}/${nativeFeature.index_path}` diff --git a/packages/snjs/lib/Services/Storage/DiskStorageService.ts b/packages/snjs/lib/Services/Storage/DiskStorageService.ts index 9620abc57..dde5e411c 100644 --- a/packages/snjs/lib/Services/Storage/DiskStorageService.ts +++ b/packages/snjs/lib/Services/Storage/DiskStorageService.ts @@ -92,7 +92,7 @@ export class DiskStorageService extends Services.AbstractService implements Serv public setEncryptionPolicy(encryptionPolicy: Services.StorageEncryptionPolicy, persist = true): void { if ( encryptionPolicy === Services.StorageEncryptionPolicy.Disabled && - ![Environment.Mobile, Environment.NativeMobileWeb].includes(this.environment) + ![Environment.Mobile].includes(this.environment) ) { throw Error('Disabling storage encryption is only available on mobile.') } diff --git a/packages/web/src/javascripts/NativeMobileWeb/MobileWebReceiver.ts b/packages/web/src/javascripts/NativeMobileWeb/MobileWebReceiver.ts index 523de9723..15a14921c 100644 --- a/packages/web/src/javascripts/NativeMobileWeb/MobileWebReceiver.ts +++ b/packages/web/src/javascripts/NativeMobileWeb/MobileWebReceiver.ts @@ -35,7 +35,7 @@ export class MobileWebReceiver { this.handleNativeEvent(nativeEvent) } } catch (error) { - console.log('Error parsing message from React Native', error) + console.log('[MobileWebReceiver] Error parsing message from React Native', error) } } diff --git a/packages/web/src/javascripts/Utils/ManageSubscription.ts b/packages/web/src/javascripts/Utils/ManageSubscription.ts index 112736533..40b412db0 100644 --- a/packages/web/src/javascripts/Utils/ManageSubscription.ts +++ b/packages/web/src/javascripts/Utils/ManageSubscription.ts @@ -8,7 +8,7 @@ export async function openSubscriptionDashboard(application: SNApplication) { const url = `${window.dashboardUrl}?subscription_token=${token}` - if (application.deviceInterface.environment === Environment.NativeMobileWeb) { + if (application.deviceInterface.environment === Environment.Mobile) { application.deviceInterface.openUrl(url) return }