Merge pull request #517 from whereby/kevinhanna/fix-safari17-handler-…

…logic media: Only use Safari17 handler when flag is eanbled
whereby · Jan 14, 2025 · 42ff0ee · 42ff0ee
2 parents e3030be + cc22bea
commit 42ff0ee
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 10 deletions.
diff --git a/.changeset/cyan-elephants-shake.md b/.changeset/cyan-elephants-shake.md
@@ -0,0 +1,5 @@
+---
+"@whereby.com/media": patch
+---
+
+Fix Safari17 Mediasoup handler selection logic
diff --git a/packages/media/src/utils/__tests__/getMediasoupDevice.spec.ts b/packages/media/src/utils/__tests__/getMediasoupDevice.spec.ts
@@ -63,13 +63,36 @@ describe("getMediasoupClient", () => {
         });
     });
 
-    describe.each(["AppleCoreMedia", "AppleWebkit", "Safari", "iphone", "ipad"])(
-        "when the userAgent matches %s",
-        (userAgent) => {
-            beforeEach(() => {
-                (global as any).userAgent.mockReturnValue(userAgent);
-            });
+    describe.each(["applecoremedia", "applewebkit", "safari"])("when the userAgent matches %s", (userAgent) => {
+        beforeEach(() => {
+            (global as any).userAgent.mockReturnValue(userAgent);
+        });
+
+        it("returns a Safari12 device", () => {
+            const factory = jest.fn();
+            Safari17.createFactory.mockImplementation(() => factory);
+
+            getMediasoupDevice({ safari17HandlerOn: false });
+
+            expect(mediasoupClient.Device).toHaveBeenCalledWith({ handlerName: "Safari12" });
+        });
+    });
 
+    describe.each(["iphone", "ipad"])("when the userAgent matches %s", (userAgent) => {
+        beforeEach(() => {
+            (global as any).userAgent.mockReturnValue(userAgent);
+        });
+
+        it("returns a Safari12 device", () => {
+            const factory = jest.fn();
+            Safari17.createFactory.mockImplementation(() => factory);
+
+            getMediasoupDevice({ safari17HandlerOn: false });
+
+            expect(mediasoupClient.Device).toHaveBeenCalledWith({ handlerName: "Safari12" });
+        });
+
+        describe("when the safari17HandlerOn feature is enabled", () => {
             it("returns a Safari17 device", () => {
                 const factory = jest.fn();
                 Safari17.createFactory.mockImplementation(() => factory);
@@ -78,6 +101,6 @@ describe("getMediasoupClient", () => {
 
                 expect(mediasoupClient.Device).toHaveBeenCalledWith({ handlerFactory: factory });
             });
-        },
-    );
+        });
+    });
 });
diff --git a/packages/media/src/utils/getMediasoupDevice.ts b/packages/media/src/utils/getMediasoupDevice.ts
@@ -12,7 +12,7 @@ export const getMediasoupDevice = (features: Record<string, boolean | undefined>
     }
 
     let handlerName: SupportedDevice =
-        detectDevice() || (/applecoremedia|applewebkit|safari/i.test(navigator.userAgent) ? "Safari17" : undefined);
+        detectDevice() || (/applecoremedia|applewebkit|safari/i.test(navigator.userAgent) ? "Safari12" : undefined);
 
     if (handlerName === "Safari12" && typeof navigator === "object" && typeof navigator.userAgent === "string") {
         const uaParser = new UAParser(navigator.userAgent);
@@ -26,7 +26,13 @@ export const getMediasoupDevice = (features: Record<string, boolean | undefined>
 
     // Since custom browsers on iOS/iPadOS are using webkit under the hood, we must use
     // the Safari handler even if detected as something else (like Chrome)
-    if (/iphone|ipad/i.test(navigator.userAgent)) handlerName = "Safari17";
+    if (/iphone|ipad/i.test(navigator.userAgent)) {
+        if (features.safari17HandlerOn) {
+            handlerName = "Safari17";
+        } else {
+            handlerName = "Safari12";
+        }
+    }
 
     if (handlerName === "Safari17") {
         // we use a custom patched version of the Safari handler that fixes simulcast bandwidth limiting