sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
77.94k stars 4.07k forks source link

select input breaks in Firefox on Windows when using `{ ...$$props }` #6625

Closed TehShrike closed 5 months ago

TehShrike commented 3 years ago

Describe the bug

We (me and @artskydj) have only been able to reproduce this bug in Firefox (90.0.2) on Windows (Windows 10 Pro).

We have not been able to reproduce the issue in Firefox (90.0.2) on macOS, or in Safari or Chrome or Edge on Windows or macOS.

In this very specific scenario, keyboard controls stop working correctly for select controls.

The issue only happens when the select element has { ...$$props } on it. If you remove the spread, or change it to { ...$$restProps } the issue does not reproduce.

Reproduction

REPL: https://svelte.dev/repl/090833a91f4c4a61bb80c1366d74d8f0?version=3.42.1

  1. set focus in the select control without expanding the dropdown
  2. Press the down arrow key on your keyboard multiple times

Expected outcome: every time you press the down arrow key on your keyboard, the next item in the control should be selected, until "delta" is selected.

Observed outcome: pressing the down arrow key on your keyboard toggles the select inconsistently between the second and third values (bravo and charlie). It seems to stay on bravo twice as long as it stays on charlie.

Logs

No response

System Info

Firefox, Windows

Severity

annoyance

TehShrike commented 3 years ago

This particular issue seems to reproduce back to Svelte 3.25 – before then, the select in that REPL seems to be broken in a different way, in all browsers.

geoffrich commented 3 years ago

This seemed really weird (and an a11y concern), so I took a little time to look into it. I don't know why this is Firefox + Windows specific, but I think the underlying issue is that you're binding to value on the <select>, but also setting the value when spreading $$props onto that same element. This is why the issue doesn't happen with $$restProps -- value is not included in that case.

If you do this without going through $$props, you get an error: "attributes need to be unique".

<!-- Warning: Attributes need to be unique -->
<select {value} bind:value>
</select>

I believe the same behavior should be disallowed when setting the props indirectly with $$props -- you shouldn't be able to set a prop and bind to it. However, this might be tricky since the compiler might not be able to detect this at compile time.

If you still want to use $$props, you can work around this by renaming the value prop to val or similar. The issue doesn't happen with the following:

<script>
    export let val;
</script>

<select {...$$props} bind:value={val}>
    <slot></slot>
</select>
dummdidumm commented 5 months ago

No longer reproducible in Firefox+Svelte4+Windows