diff --git a/.changeset/popular-feet-rule.md b/.changeset/popular-feet-rule.md new file mode 100644 index 0000000000..fee598af45 --- /dev/null +++ b/.changeset/popular-feet-rule.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: make media bindings more robust diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/media.js b/packages/svelte/src/internal/client/dom/elements/bindings/media.js index b7708b4f34..320e3856f2 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/media.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/media.js @@ -22,7 +22,8 @@ function time_ranges_to_array(ranges) { export function bind_current_time(media, get_value, update) { /** @type {number} */ var raf_id; - var updating = false; + /** @type {number} */ + var value; // Ideally, listening to timeupdate would be enough, but it fires too infrequently for the currentTime // binding, which is why we use a raf loop, too. We additionally still listen to timeupdate because @@ -34,22 +35,21 @@ export function bind_current_time(media, get_value, update) { raf_id = requestAnimationFrame(callback); } - updating = true; - update(media.currentTime); + var next_value = media.currentTime; + if (value !== next_value) { + update((value = next_value)); + } }; raf_id = requestAnimationFrame(callback); media.addEventListener('timeupdate', callback); render_effect(() => { - var value = get_value(); + var next_value = Number(get_value()); - // through isNaN we also allow number strings, which is more robust - if (!updating && !isNaN(/** @type {any} */ (value))) { - media.currentTime = /** @type {number} */ (value); + if (value !== next_value && !isNaN(/** @type {any} */ (next_value))) { + media.currentTime = value = next_value; } - - updating = false; }); teardown(() => cancelAnimationFrame(raf_id)); @@ -113,22 +113,21 @@ export function bind_ready_state(media, update) { * @param {(playback_rate: number) => void} update */ export function bind_playback_rate(media, get_value, update) { - var updating = false; - - // Needs to happen after the element is inserted into the dom, else playback will be set back to 1 by the browser. - // For hydration we could do it immediately but the additional code is not worth the lost microtask. + // Needs to happen after element is inserted into the dom (which is guaranteed by using effect), + // else playback will be set back to 1 by the browser effect(() => { - var value = get_value(); + var value = Number(get_value()); - // through isNaN we also allow number strings, which is more robust - if (!isNaN(/** @type {any} */ (value)) && value !== media.playbackRate) { - updating = true; - media.playbackRate = /** @type {number} */ (value); + if (value !== media.playbackRate && !isNaN(value)) { + media.playbackRate = value; } + }); + // Start listening to ratechange events after the element is inserted into the dom, + // else playback will be set to 1 by the browser + effect(() => { listen(media, ['ratechange'], () => { - if (!updating) update(media.playbackRate); - updating = false; + update(media.playbackRate); }); }); } @@ -200,9 +199,7 @@ export function bind_paused(media, get_value, update) { * @param {(volume: number) => void} update */ export function bind_volume(media, get_value, update) { - var updating = false; var callback = () => { - updating = true; update(media.volume); }; @@ -213,14 +210,11 @@ export function bind_volume(media, get_value, update) { listen(media, ['volumechange'], callback, false); render_effect(() => { - var value = get_value(); + var value = Number(get_value()); - // through isNaN we also allow number strings, which is more robust - if (!updating && !isNaN(/** @type {any} */ (value))) { - media.volume = /** @type {number} */ (value); + if (value !== media.volume && !isNaN(value)) { + media.volume = value; } - - updating = false; }); } @@ -230,10 +224,7 @@ export function bind_volume(media, get_value, update) { * @param {(muted: boolean) => void} update */ export function bind_muted(media, get_value, update) { - var updating = false; - var callback = () => { - updating = true; update(media.muted); }; @@ -244,9 +235,8 @@ export function bind_muted(media, get_value, update) { listen(media, ['volumechange'], callback, false); render_effect(() => { - var value = get_value(); + var value = !!get_value(); - if (!updating) media.muted = !!value; - updating = false; + if (media.muted !== value) media.muted = value; }); } diff --git a/packages/svelte/tests/runtime-browser/assert.js b/packages/svelte/tests/runtime-browser/assert.js index c2c265d90f..c30467279c 100644 --- a/packages/svelte/tests/runtime-browser/assert.js +++ b/packages/svelte/tests/runtime-browser/assert.js @@ -44,6 +44,7 @@ export function equal(a, b, message) { /** * @param {any} condition * @param {string} [message] + * @returns {asserts condition} */ export function ok(condition, message) { if (!condition) throw new Error(message || `Expected ${condition} to be truthy`); diff --git a/packages/svelte/tests/runtime-browser/samples/bind-muted/_config.js b/packages/svelte/tests/runtime-browser/samples/bind-muted/_config.js new file mode 100644 index 0000000000..cfc558a9c3 --- /dev/null +++ b/packages/svelte/tests/runtime-browser/samples/bind-muted/_config.js @@ -0,0 +1,29 @@ +import { test, ok } from '../../assert'; + +export default test({ + mode: ['client'], + async test({ assert, target }) { + const audio = target.querySelector('audio'); + const button = target.querySelector('button'); + ok(audio); + + assert.equal(audio.muted, false); + + audio.muted = true; + audio.dispatchEvent(new CustomEvent('volumechange')); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.muted, true, 'event'); + + button?.click(); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.muted, false, 'click 1'); + + button?.click(); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.muted, true, 'click 2'); + + button?.click(); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.muted, false, 'click 3'); + } +}); diff --git a/packages/svelte/tests/runtime-browser/samples/bind-muted/main.svelte b/packages/svelte/tests/runtime-browser/samples/bind-muted/main.svelte new file mode 100644 index 0000000000..ae03256e95 --- /dev/null +++ b/packages/svelte/tests/runtime-browser/samples/bind-muted/main.svelte @@ -0,0 +1,6 @@ + + + + diff --git a/packages/svelte/tests/runtime-browser/samples/bind-playbackrate/_config.js b/packages/svelte/tests/runtime-browser/samples/bind-playbackrate/_config.js new file mode 100644 index 0000000000..c2718e2850 --- /dev/null +++ b/packages/svelte/tests/runtime-browser/samples/bind-playbackrate/_config.js @@ -0,0 +1,29 @@ +import { test, ok } from '../../assert'; + +export default test({ + mode: ['client'], + async test({ assert, target }) { + const audio = target.querySelector('audio'); + const button = target.querySelector('button'); + ok(audio); + + assert.equal(audio.playbackRate, 0.5); + + audio.playbackRate = 1.0; + audio.dispatchEvent(new CustomEvent('ratechange')); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.playbackRate, 1.0); + + button?.click(); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.playbackRate, 2); + + button?.click(); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.playbackRate, 3); + + button?.click(); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.playbackRate, 4); + } +}); diff --git a/packages/svelte/tests/runtime-browser/samples/bind-playbackrate/main.svelte b/packages/svelte/tests/runtime-browser/samples/bind-playbackrate/main.svelte new file mode 100644 index 0000000000..07c2080637 --- /dev/null +++ b/packages/svelte/tests/runtime-browser/samples/bind-playbackrate/main.svelte @@ -0,0 +1,6 @@ + + + + diff --git a/packages/svelte/tests/runtime-browser/samples/bind-volume/_config.js b/packages/svelte/tests/runtime-browser/samples/bind-volume/_config.js new file mode 100644 index 0000000000..61f3db9692 --- /dev/null +++ b/packages/svelte/tests/runtime-browser/samples/bind-volume/_config.js @@ -0,0 +1,29 @@ +import { test, ok } from '../../assert'; + +export default test({ + mode: ['client'], + async test({ assert, target }) { + const audio = target.querySelector('audio'); + const button = target.querySelector('button'); + ok(audio); + + assert.equal(audio.volume, 0.1); + + audio.volume = 0.2; + audio.dispatchEvent(new CustomEvent('volumechange')); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.volume, 0.2); + + button?.click(); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.volume, 0.2 + 0.1); // JavaScript can't add floating point numbers correctly + + button?.click(); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.volume, 0.2 + 0.1 + 0.1); + + button?.click(); + await new Promise((r) => setTimeout(r, 100)); + assert.equal(audio.volume, 0.2 + 0.1 + 0.1 + 0.1); + } +}); diff --git a/packages/svelte/tests/runtime-browser/samples/bind-volume/main.svelte b/packages/svelte/tests/runtime-browser/samples/bind-volume/main.svelte new file mode 100644 index 0000000000..21755e0cba --- /dev/null +++ b/packages/svelte/tests/runtime-browser/samples/bind-volume/main.svelte @@ -0,0 +1,6 @@ + + + +