From fb9948c79cca586dc71b37747e287dff3e588452 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 12 Jul 2023 09:38:25 +0100 Subject: [PATCH] DEV: Make capabilities available outside of application instance (#22516) Browser capabilities are inherently unconnected to the lifecycle of our app. Making them formally available outside of the service means that they can safely be used in non-app-linked functions without needing risky hacks like `helperContext()` or `discourse-common/lib/get-owner`. One example of where the old hacks were problematic is the `translateModKey()` utility function. This is called in the root of the `discourse/components/modal/keyboard-shortcuts-help` es6 module. If anything (e.g. a theme/plugin) caused that es6 module to be `require()`d before the application was booted, a fatal error would occur. Following this commit, `translateModKey()` can safely import and access `capabilities` without needing to worry about the app lifecycle. The only potential downside to this approach is that the capabilities data now persists across tests. If any tests need to 'stub' capabilities, they will need to revert their changes at the end of the test (e.g. by using Sinon to stub a property). This commit also updates some legacy references from `capabilities:main` to `service:capabilities`. --- .../discourse/app/lib/keyboard-shortcuts.js | 6 +- .../discourse/app/lib/utilities.js | 6 +- .../discourse/app/services/capabilities.js | 81 ++++++++++--------- .../discourse/app/services/composer.js | 5 +- .../tests/acceptance/sidebar-user-test.js | 3 +- .../discourse/lib/chat-message-interactor.js | 7 +- 6 files changed, 55 insertions(+), 53 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js b/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js index 7457b705b1f..a5cbc9e267b 100644 --- a/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js +++ b/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js @@ -13,7 +13,7 @@ import domUtils from "discourse-common/utils/dom-utils"; import { INPUT_DELAY } from "discourse-common/config/environment"; import { ajax } from "discourse/lib/ajax"; import { headerOffset } from "discourse/lib/offset-calculator"; -import { helperContext } from "discourse-common/lib/helpers"; +import { capabilities } from "discourse/services/capabilities"; let extraKeyboardShortcutsHelp = {}; function addExtraKeyboardShortcutHelp(help) { @@ -876,13 +876,13 @@ export default { }, webviewKeyboardBack() { - if (helperContext().capabilities.isAppWebview) { + if (capabilities.isAppWebview) { window.history.back(); } }, webviewKeyboardForward() { - if (helperContext().capabilities.isAppWebview) { + if (capabilities.isAppWebview) { window.history.forward(); } }, diff --git a/app/assets/javascripts/discourse/app/lib/utilities.js b/app/assets/javascripts/discourse/app/lib/utilities.js index cd825625948..6fb79b524fc 100644 --- a/app/assets/javascripts/discourse/app/lib/utilities.js +++ b/app/assets/javascripts/discourse/app/lib/utilities.js @@ -2,10 +2,10 @@ import getURL from "discourse-common/lib/get-url"; import Handlebars from "handlebars"; import I18n from "I18n"; import { escape } from "pretty-text/sanitizer"; -import { helperContext } from "discourse-common/lib/helpers"; import toMarkdown from "discourse/lib/to-markdown"; import deprecated from "discourse-common/lib/deprecated"; import * as AvatarUtils from "discourse-common/lib/avatar-utils"; +import { capabilities } from "discourse/services/capabilities"; let _defaultHomepage; @@ -268,7 +268,7 @@ export function determinePostReplaceSelection({ export function isAppleDevice() { // IE has no DOMNodeInserted so can not get this hack despite saying it is like iPhone // This will apply hack on all iDevices - let caps = helperContext().capabilities; + let caps = capabilities; return caps.isIOS && !window.navigator.userAgent.match(/Trident/g); } @@ -451,7 +451,7 @@ export function modKeysPressed(event) { } export function translateModKey(string) { - const { isApple } = helperContext().capabilities; + const { isApple } = capabilities; // Apple device users are used to glyphs for shortcut keys if (isApple) { string = string diff --git a/app/assets/javascripts/discourse/app/services/capabilities.js b/app/assets/javascripts/discourse/app/services/capabilities.js index aca1f7c1cd7..41dac2d3607 100644 --- a/app/assets/javascripts/discourse/app/services/capabilities.js +++ b/app/assets/javascripts/discourse/app/services/capabilities.js @@ -1,47 +1,54 @@ -import Service from "@ember/service"; - const APPLE_NAVIGATOR_PLATFORMS = /iPhone|iPod|iPad|Macintosh|MacIntel/; const APPLE_USER_AGENT_DATA_PLATFORM = /macOS/; -// Lets us know about browser's capabilities -export default class Capabilities extends Service { - constructor() { - super(...arguments); +function calculateCapabilities() { + const capabilities = {}; - const ua = navigator.userAgent; + const ua = navigator.userAgent; - this.touch = navigator.maxTouchPoints > 1 || "ontouchstart" in window; + capabilities.touch = navigator.maxTouchPoints > 1 || "ontouchstart" in window; - this.isAndroid = ua.includes("Android"); - this.isWinphone = ua.includes("Windows Phone"); - this.isIpadOS = - ua.includes("Mac OS") && !/iPhone|iPod/.test(ua) && this.touch; - this.isIOS = - (/iPhone|iPod/.test(navigator.userAgent) || this.isIpadOS) && - !window.MSStream; - this.isApple = - APPLE_NAVIGATOR_PLATFORMS.test(navigator.platform) || - (navigator.userAgentData && - APPLE_USER_AGENT_DATA_PLATFORM.test(navigator.userAgentData.platform)); + capabilities.isAndroid = ua.includes("Android"); + capabilities.isWinphone = ua.includes("Windows Phone"); + capabilities.isIpadOS = + ua.includes("Mac OS") && !/iPhone|iPod/.test(ua) && capabilities.touch; + capabilities.isIOS = + (/iPhone|iPod/.test(navigator.userAgent) || capabilities.isIpadOS) && + !window.MSStream; + capabilities.isApple = + APPLE_NAVIGATOR_PLATFORMS.test(navigator.platform) || + (navigator.userAgentData && + APPLE_USER_AGENT_DATA_PLATFORM.test(navigator.userAgentData.platform)); - this.isOpera = !!window.opera || ua.includes(" OPR/"); - this.isFirefox = ua.includes("Firefox"); - this.isChrome = !!window.chrome && !this.isOpera; - this.isSafari = - /Constructor/.test(window.HTMLElement) || - window.safari?.pushNotification?.toString() === - "[object SafariRemoteNotification]"; + capabilities.isOpera = !!window.opera || ua.includes(" OPR/"); + capabilities.isFirefox = ua.includes("Firefox"); + capabilities.isChrome = !!window.chrome && !capabilities.isOpera; + capabilities.isSafari = + /Constructor/.test(window.HTMLElement) || + window.safari?.pushNotification?.toString() === + "[object SafariRemoteNotification]"; - this.hasContactPicker = - "contacts" in navigator && "ContactsManager" in window; - this.canVibrate = "vibrate" in navigator; - this.isPwa = - window.matchMedia("(display-mode: standalone)").matches || - window.navigator.standalone || - document.referrer.includes("android-app://"); - this.isiOSPWA = this.isPwa && this.isIOS; - this.wasLaunchedFromDiscourseHub = - window.location.search.includes("discourse_app=1"); - this.isAppWebview = window.ReactNativeWebView !== undefined; + capabilities.hasContactPicker = + "contacts" in navigator && "ContactsManager" in window; + capabilities.canVibrate = "vibrate" in navigator; + capabilities.isPwa = + window.matchMedia("(display-mode: standalone)").matches || + window.navigator.standalone || + document.referrer.includes("android-app://"); + capabilities.isiOSPWA = capabilities.isPwa && capabilities.isIOS; + capabilities.wasLaunchedFromDiscourseHub = + window.location.search.includes("discourse_app=1"); + capabilities.isAppWebview = window.ReactNativeWebView !== undefined; + + return capabilities; +} + +export const capabilities = calculateCapabilities(); + +export default class CapabilitiesService { + static isServiceFactory = true; + + static create() { + return capabilities; } } diff --git a/app/assets/javascripts/discourse/app/services/composer.js b/app/assets/javascripts/discourse/app/services/composer.js index 06049924d30..7a95da9a5fb 100644 --- a/app/assets/javascripts/discourse/app/services/composer.js +++ b/app/assets/javascripts/discourse/app/services/composer.js @@ -100,6 +100,7 @@ export default class ComposerController extends Controller { @service site; @service store; @service appEvents; + @service capabilities; checkedMessages = false; messageCount = null; @@ -128,10 +129,6 @@ export default class ComposerController extends Controller { return getOwner(this).lookup("controller:topic"); } - get capabilities() { - return getOwner(this).lookup("capabilities:main"); - } - @on("init") _setupPreview() { const val = this.site.mobileView diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-test.js index b92fb0e1c3e..2bb6c7858ff 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-test.js @@ -7,6 +7,7 @@ import { query, updateCurrentUser, } from "discourse/tests/helpers/qunit-helpers"; +import Sinon from "sinon"; acceptance( "Sidebar - Logged on user - Legacy navigation menu enabled", @@ -162,7 +163,7 @@ acceptance( test("button to toggle between mobile and desktop view on touch devices ", async function (assert) { const capabilities = this.container.lookup("service:capabilities"); - capabilities.touch = true; + Sinon.stub(capabilities, "touch").value(true); await visit("/"); diff --git a/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js b/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js index bc5da78b96a..d83077b93a2 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js +++ b/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js @@ -12,7 +12,7 @@ import { clipboardCopy } from "discourse/lib/utilities"; import ChatMessageReaction, { REACTIONS, } from "discourse/plugins/chat/discourse/models/chat-message-reaction"; -import { getOwner, setOwner } from "@ember/application"; +import { setOwner } from "@ember/application"; import { tracked } from "@glimmer/tracking"; import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; import { MESSAGE_CONTEXT_THREAD } from "discourse/plugins/chat/discourse/components/chat-message"; @@ -42,6 +42,7 @@ export default class ChatMessageInteractor { @service currentUser; @service site; @service router; + @service capabilities; @tracked message = null; @tracked context = null; @@ -56,10 +57,6 @@ export default class ChatMessageInteractor { this.cachedFavoritesReactions = this.chatEmojiReactionStore.favorites; } - get capabilities() { - return getOwner(this).lookup("capabilities:main"); - } - get pane() { return this.context === MESSAGE_CONTEXT_THREAD ? this.chatThreadPane