fix(player): correctly append search params to download url

closes #1409
vidstack · Sep 15, 2024 · 39774ab · 39774ab
1 parent 4734549
commit 39774ab
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 17 deletions.
diff --git a/packages/react/src/components/layouts/default/ui/buttons.tsx b/packages/react/src/components/layouts/default/ui/buttons.tsx
@@ -1,9 +1,10 @@
 import * as React from 'react';
 
-import { uppercaseFirstChar } from 'maverick.js/std';
+import { isString, uppercaseFirstChar } from 'maverick.js/std';
 import { getDownloadFile, isTrackCaptionKind } from 'vidstack';
 
 import { useMediaState } from '../../../../hooks/use-media-state';
+import { appendParamsToURL } from '../../../../utils';
 import { AirPlayButton } from '../../../ui/buttons/airplay-button';
 import { CaptionButton } from '../../../ui/buttons/caption-button';
 import { FullscreenButton } from '../../../ui/buttons/fullscreen-button';
@@ -280,13 +281,13 @@ function DefaultDownloadButton() {
     }),
     downloadText = useDefaultLayoutWord('Download');
 
-  return file ? (
+  return isString(file?.url) ? (
     <DefaultTooltip content={downloadText} placement="top">
       <a
         role="button"
         className="vds-download-button vds-button"
         aria-label={downloadText}
-        href={file.url + `?download=${file.name}`}
+        href={appendParamsToURL(file.url, { download: file.name })}
         download={file.name}
         target="_blank"
       >

diff --git a/packages/react/src/components/layouts/plyr/layout.tsx b/packages/react/src/components/layouts/plyr/layout.tsx
@@ -2,7 +2,7 @@ import * as React from 'react';
 
 import { signal } from 'maverick.js';
 import { composeRefs, useSignal } from 'maverick.js/react';
-import { isNumber, listenEvent } from 'maverick.js/std';
+import { isNumber, isString, listenEvent } from 'maverick.js/std';
 import type { VTTCue } from 'media-captions';
 import {
   isKeyboardClick,
@@ -23,6 +23,7 @@ import { useMediaContext } from '../../../hooks/use-media-context';
 import { useMediaRemote } from '../../../hooks/use-media-remote';
 import { useMediaState } from '../../../hooks/use-media-state';
 import { isRemotionSource } from '../../../providers/remotion';
+import { appendParamsToURL } from '../../../utils';
 import { Primitive, type PrimitivePropsWithRef } from '../../primitives/nodes';
 import { AirPlayButton } from '../../ui/buttons/airplay-button';
 import { CaptionButton } from '../../ui/buttons/caption-button';
@@ -746,10 +747,10 @@ function PlyrDownloadButton() {
 
   return slot(
     'download',
-    file ? (
+    isString(file?.url) ? (
       <a
         className="plyr__controls__item plyr__control"
-        href={file.url + `?download=${file.name}`}
+        href={appendParamsToURL(file.url, { download: file.name })}
         download={file.name}
         target="_blank"
       >

diff --git a/packages/react/src/utils.ts b/packages/react/src/utils.ts
@@ -15,3 +15,13 @@ export function createVTTCue(startTime = 0, endTime = 0, text = ''): VTTCue {
 
   return new window.VTTCue(startTime, endTime, text);
 }
+
+export function appendParamsToURL(baseUrl: string, params: Record<string, any>) {
+  const url = new URL(baseUrl);
+
+  for (const key of Object.keys(params)) {
+    url.searchParams.set(key, params[key] + '');
+  }
+
+  return url.toString();
+}
diff --git a/packages/vidstack/src/elements/define/layouts/default/ui/buttons.ts b/packages/vidstack/src/elements/define/layouts/default/ui/buttons.ts
@@ -1,12 +1,12 @@
 import { html } from 'lit-html';
 import { ref as $ref, type RefOrCallback } from 'lit-html/directives/ref.js';
-import { isNil, noop, uppercaseFirstChar } from 'maverick.js/std';
+import { isNil, isString, noop, uppercaseFirstChar } from 'maverick.js/std';
 
 import { useDefaultLayoutContext } from '../../../../../components/layouts/default/context';
 import { i18n } from '../../../../../components/layouts/default/translations';
 import type { TooltipPlacement } from '../../../../../components/ui/tooltip/tooltip-content';
 import { useMediaState } from '../../../../../core/api/media-context';
-import { getDownloadFile } from '../../../../../utils/network';
+import { appendParamsToURL, getDownloadFile } from '../../../../../utils/network';
 import { $signal } from '../../../../lit/directives/signal';
 import { IconSlot, IconSlots } from '../slots';
 import { $i18n } from './utils';
@@ -232,15 +232,15 @@ export function DefaultDownloadButton() {
         download: $download,
       });
 
-    return file
+    return isString(file?.url)
       ? html`
           <media-tooltip class="vds-download-tooltip vds-tooltip">
             <media-tooltip-trigger>
               <a
                 role="button"
                 class="vds-download-button vds-button"
                 aria-label=${$i18n(translations, 'Download')}
-                href=${file.url + `?download=${file.name}`}
+                href=${appendParamsToURL(file.url, { download: file.name })}
                 download=${file.name}
                 target="_blank"
               >

diff --git a/packages/vidstack/src/elements/define/layouts/plyr/ui.ts b/packages/vidstack/src/elements/define/layouts/plyr/ui.ts
@@ -1,7 +1,7 @@
 import { html, type TemplateResult } from 'lit-html';
 import { unsafeHTML } from 'lit-html/directives/unsafe-html.js';
 import { computed, effect, peek, signal, type ReadSignal } from 'maverick.js';
-import { isKeyboardClick, isKeyboardEvent, listenEvent } from 'maverick.js/std';
+import { isKeyboardClick, isKeyboardEvent, isString, listenEvent } from 'maverick.js/std';
 import type { VTTCue } from 'media-captions';
 
 import {
@@ -12,7 +12,7 @@ import type { PlyrControl, PlyrMarker } from '../../../../components/layouts/ply
 import { i18n, type PlyrLayoutWord } from '../../../../components/layouts/plyr/translations';
 import { useMediaContext } from '../../../../core/api/media-context';
 import type { MediaSeekingRequestEvent } from '../../../../core/api/media-request-events';
-import { getDownloadFile } from '../../../../utils/network';
+import { appendParamsToURL, getDownloadFile } from '../../../../utils/network';
 import { $signal } from '../../../lit/directives/signal';
 
 export function PlyrAudioLayout() {
@@ -464,11 +464,11 @@ function DownloadButton() {
       }),
       $downloadText = $i18n(translations, 'Download');
 
-    return file
+    return isString(file?.url)
       ? html`
           <a
             class="plyr__controls__item plyr__control"
-            href=${file.url + `?download=${file.name}`}
+            href=${appendParamsToURL(file.url, { download: file.name })}
             download=${file.name}
             target="_blank"
           >

diff --git a/packages/vidstack/src/utils/network.ts b/packages/vidstack/src/utils/network.ts
@@ -10,13 +10,13 @@ import type { Src } from '../core/api/src-types';
 import { isAudioSrc, isVideoSrc } from './mime';
 
 export function appendParamsToURL(baseUrl: string, params: Record<string, any>) {
-  const searchParams = new URLSearchParams();
+  const url = new URL(baseUrl);
 
   for (const key of Object.keys(params)) {
-    searchParams.set(key, params[key] + '');
+    url.searchParams.set(key, params[key] + '');
   }
 
-  return baseUrl + '?' + searchParams.toString();
+  return url.toString();
 }
 
 export function preconnect(