From 1b9c4a9c11f636000d2927ff440a6616ddfe3c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Tue, 25 Apr 2023 15:20:17 +0200 Subject: [PATCH] [PM-1829] Allow Safari popup to be full height in Safari >= 16.1 (#5226) * Logic for calculating if we should apply height fix for safari * Revert safari height fix for safari >= 16.1 * Deprecate static isBrowser funcions, using them directly is incorrect * Changes based on feedback * Update CSS to use & selector --------- Co-authored-by: Hinton --- apps/browser/src/popup/main.ts | 8 +++ apps/browser/src/popup/scss/environment.scss | 16 +++-- .../browserPlatformUtils.service.spec.ts | 71 +++++++++++++++++++ .../services/browserPlatformUtils.service.ts | 58 ++++++++++++--- 4 files changed, 137 insertions(+), 16 deletions(-) diff --git a/apps/browser/src/popup/main.ts b/apps/browser/src/popup/main.ts index b550cba86f..3103fa2d99 100644 --- a/apps/browser/src/popup/main.ts +++ b/apps/browser/src/popup/main.ts @@ -1,10 +1,18 @@ import { enableProdMode } from "@angular/core"; import { platformBrowserDynamic } from "@angular/platform-browser-dynamic"; +import BrowserPlatformUtilsService from "../services/browserPlatformUtils.service"; + require("./scss/popup.scss"); import { AppModule } from "./app.module"; +// We put this first to minimize the delay in window changing. +// Should be removed once we deprecate support for Safari 16.0 and older. See Jira ticket [PM-1861] +if (BrowserPlatformUtilsService.shouldApplySafariHeightFix(window)) { + document.documentElement.classList.add("safari_height_fix"); +} + if (process.env.ENV === "production") { enableProdMode(); } diff --git a/apps/browser/src/popup/scss/environment.scss b/apps/browser/src/popup/scss/environment.scss index cbd4d8ad6d..042bcd1b45 100644 --- a/apps/browser/src/popup/scss/environment.scss +++ b/apps/browser/src/popup/scss/environment.scss @@ -1,15 +1,17 @@ @import "variables.scss"; html.browser_safari { - body { - height: 360px !important; + &.safari_height_fix { + body { + height: 360px !important; - &.body-xs { - height: 300px !important; - } + &.body-xs { + height: 300px !important; + } - &.body-full { - height: 100% !important; + &.body-full { + height: 100% !important; + } } } diff --git a/apps/browser/src/services/browserPlatformUtils.service.spec.ts b/apps/browser/src/services/browserPlatformUtils.service.spec.ts index 42e72052b2..c0d62b4d99 100644 --- a/apps/browser/src/services/browserPlatformUtils.service.spec.ts +++ b/apps/browser/src/services/browserPlatformUtils.service.spec.ts @@ -22,6 +22,7 @@ describe("Browser Utils Service", () => { afterEach(() => { window.matchMedia = undefined; (window as any).chrome = undefined; + (BrowserPlatformUtilsService as any).deviceCache = null; }); it("should detect chrome", () => { @@ -86,3 +87,73 @@ describe("Browser Utils Service", () => { }); }); }); + +describe("Safari Height Fix", () => { + const originalUserAgent = navigator.userAgent; + + // Reset the userAgent. + afterAll(() => { + Object.defineProperty(navigator, "userAgent", { + value: originalUserAgent, + }); + }); + + afterEach(() => { + (BrowserPlatformUtilsService as any).deviceCache = null; + }); + + test.each([ + [ + "safari 15.6.1", + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.6.1 Safari/605.1.15", + true, + ], + [ + "safari 16.0", + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.0 Safari/605.1.15", + true, + ], + + [ + "safari 16.1", + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.1 Safari/605.1.15", + false, + ], + [ + "safari 16.4", + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.4 Safari/605.1.15", + false, + ], + [ + "safari 17.0 (future release)", + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Safari/605.1.15", + false, + ], + [ + "chrome", + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36", + false, + ], + [ + "firefox", + "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0", + false, + ], + [ + "opera", + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3175.3 Safari/537.36 OPR/49.0.2695.0 (Edition developer)", + false, + ], + [ + "edge", + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.74 Safari/537.36 Edg/79.0.309.43", + false, + ], + ])("Apply fix for %s", (name, userAgent, expected) => { + Object.defineProperty(navigator, "userAgent", { + configurable: true, + value: userAgent, + }); + expect(BrowserPlatformUtilsService.shouldApplySafariHeightFix(window)).toBe(expected); + }); +}); diff --git a/apps/browser/src/services/browserPlatformUtils.service.ts b/apps/browser/src/services/browserPlatformUtils.service.ts index 08da886707..ce82bf6f25 100644 --- a/apps/browser/src/services/browserPlatformUtils.service.ts +++ b/apps/browser/src/services/browserPlatformUtils.service.ts @@ -13,7 +13,7 @@ export default class BrowserPlatformUtilsService implements PlatformUtilsService number, { tryResolve: (canceled: boolean, password: string) => Promise; date: Date } >(); - private deviceCache: DeviceType = null; + private static deviceCache: DeviceType = null; constructor( private messagingService: MessagingService, @@ -22,28 +22,32 @@ export default class BrowserPlatformUtilsService implements PlatformUtilsService private win: Window & typeof globalThis ) {} - getDevice(): DeviceType { + static getDevice(win: Window & typeof globalThis): DeviceType { if (this.deviceCache) { return this.deviceCache; } if (BrowserPlatformUtilsService.isFirefox()) { this.deviceCache = DeviceType.FirefoxExtension; - } else if (BrowserPlatformUtilsService.isOpera(this.win)) { + } else if (BrowserPlatformUtilsService.isOpera(win)) { this.deviceCache = DeviceType.OperaExtension; } else if (BrowserPlatformUtilsService.isEdge()) { this.deviceCache = DeviceType.EdgeExtension; } else if (BrowserPlatformUtilsService.isVivaldi()) { this.deviceCache = DeviceType.VivaldiExtension; - } else if (BrowserPlatformUtilsService.isChrome(this.win)) { + } else if (BrowserPlatformUtilsService.isChrome(win)) { this.deviceCache = DeviceType.ChromeExtension; - } else if (BrowserPlatformUtilsService.isSafari(this.win)) { + } else if (BrowserPlatformUtilsService.isSafari(win)) { this.deviceCache = DeviceType.SafariExtension; } return this.deviceCache; } + getDevice(): DeviceType { + return BrowserPlatformUtilsService.getDevice(this.win); + } + getDeviceString(): string { const device = DeviceType[this.getDevice()].toLowerCase(); return device.replace("extension", ""); @@ -53,6 +57,9 @@ export default class BrowserPlatformUtilsService implements PlatformUtilsService return ClientType.Browser; } + /** + * @deprecated Do not call this directly, use getDevice() instead + */ static isFirefox(): boolean { return ( navigator.userAgent.indexOf(" Firefox/") !== -1 || @@ -64,7 +71,10 @@ export default class BrowserPlatformUtilsService implements PlatformUtilsService return this.getDevice() === DeviceType.FirefoxExtension; } - static isChrome(win: Window & typeof globalThis): boolean { + /** + * @deprecated Do not call this directly, use getDevice() instead + */ + private static isChrome(win: Window & typeof globalThis): boolean { return win.chrome && navigator.userAgent.indexOf(" Chrome/") !== -1; } @@ -72,7 +82,10 @@ export default class BrowserPlatformUtilsService implements PlatformUtilsService return this.getDevice() === DeviceType.ChromeExtension; } - static isEdge(): boolean { + /** + * @deprecated Do not call this directly, use getDevice() instead + */ + private static isEdge(): boolean { return navigator.userAgent.indexOf(" Edg/") !== -1; } @@ -80,7 +93,10 @@ export default class BrowserPlatformUtilsService implements PlatformUtilsService return this.getDevice() === DeviceType.EdgeExtension; } - static isOpera(win: Window & typeof globalThis): boolean { + /** + * @deprecated Do not call this directly, use getDevice() instead + */ + private static isOpera(win: Window & typeof globalThis): boolean { return ( (!!win.opr && !!win.opr.addons) || !!win.opera || navigator.userAgent.indexOf(" OPR/") >= 0 ); @@ -90,7 +106,10 @@ export default class BrowserPlatformUtilsService implements PlatformUtilsService return this.getDevice() === DeviceType.OperaExtension; } - static isVivaldi(): boolean { + /** + * @deprecated Do not call this directly, use getDevice() instead + */ + private static isVivaldi(): boolean { return navigator.userAgent.indexOf(" Vivaldi/") !== -1; } @@ -98,6 +117,9 @@ export default class BrowserPlatformUtilsService implements PlatformUtilsService return this.getDevice() === DeviceType.VivaldiExtension; } + /** + * @deprecated Do not call this directly, use getDevice() instead + */ static isSafari(win: Window & typeof globalThis): boolean { // Opera masquerades as Safari, so make sure we're not there first return ( @@ -105,6 +127,24 @@ export default class BrowserPlatformUtilsService implements PlatformUtilsService ); } + private static safariVersion(): string { + return navigator.userAgent.match("Version/([0-9.]*)")?.[1]; + } + + /** + * Safari previous to version 16.1 had a bug which caused artifacts on hover in large extension popups. + * https://bugs.webkit.org/show_bug.cgi?id=218704 + */ + static shouldApplySafariHeightFix(win: Window & typeof globalThis): boolean { + if (BrowserPlatformUtilsService.getDevice(win) !== DeviceType.SafariExtension) { + return false; + } + + const version = BrowserPlatformUtilsService.safariVersion(); + const parts = version?.split(".")?.map((v) => Number(v)); + return parts?.[0] < 16 || (parts?.[0] === 16 && parts?.[1] === 0); + } + isSafari(): boolean { return this.getDevice() === DeviceType.SafariExtension; }