Fix: prevent form submission breaking language selector (#6598)

[prvshkmr]

Summary

This PR fixes issue #6598, where pressing Enter inside the language selector modal causes the modal to get stuck on an endless loading spinner.

Root Cause

For logged-in users, the language selector loads a <form> inside the select_language_list Turbo Frame.
Pressing Enter triggers an unintended form submission, but the server response is not Turbo-frame compatible, so Turbo cannot replace the frame content.

As a result:

• the Turbo Frame stays in the "loading" state,
• reopening the modal shows the spinner forever,
• only a full page refresh restores normal behavior.

Logged-out users do not see this bug because their modal contains no form.

Fix

This change prevents the Enter key from causing a form submission inside the language selector modal.
Avoiding the submission ensures the Turbo Frame continues to behave normally and reloads the language list as intended.

Testing

• Reproduced issue locally using Docker environment.
• Verified that the spinner freeze no longer occurs when pressing Enter.
• Confirmed that both logged-in and logged-out behaviors remain correct.
• No regressions observed in language selection or modal behavior.

Additional Notes

• Change follows project coding standards (ESLint for JS).
• Commit history is clean and scoped to the fix.
• Does not modify any localization files except en.yml if necessary (none needed here).

[HolgerJeromin]

We should not introduce handling keyCode.

https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode

[prvshkmr]

Thanks for pointing that out — you're right, keyCode is deprecated and shouldn't be introduced here.

I’ll update the PR to remove the keyCode check and switch to using event.key, which is the recommended modern approach.

Thanks for the guidance!

[tomhughes]

Is there really not a simpler way of blocking enter in the search field than this?

This is actually blocking all enter presses in the dialog unless they're on one of the language entries but I believe it's only the input field that's actually a problem so maybe we can just put something on that field?

If it does need to be done by intercepting keydown do we really need to check both the name and the code?

[tomhughes]

Can I also ask if AI was involved in writing this?

[prvshkmr]

Thanks for raising these points — they helped me re-evaluate the approach.

• You're right that there's a simpler way to handle this. The actual issue only happens when the search input submits the form, so there's no need to block Enter across the entire dialog.
• It makes more sense to attach the handler directly to the search input instead of intercepting keydown at the dialog level. That keeps the fix much more focused.
• And yes, checking both the key name and the code isn’t necessary. Using event.key === "Enter" is enough, and avoids deprecated APIs like keyCode.

I'll update the PR so that it only blocks Enter on the search field and uses a clean, modern key check.

[prvshkmr]

Can I also ask if AI was involved in writing this?

I did use AI while investigating the issue, mainly to double-check my understanding of what was going on with Turbo and the modal behavior. The actual implementation is mine, and I’m adjusting it based on the feedback here.

Happy to refine the approach further if needed. Thanks for the review.

[pablobm]

Checking for specific key presses can lead to other issues, accessibility being a common example.

The issue is with the form submitting at the wrong moment. Therefore it's probably the submission that should be tackled. There's an event for that.

I played a bit with this today. I can't tell this is 100% the best option, wondering what others think:

diff --git a/app/assets/javascripts/language_selector.js b/app/assets/javascripts/language_selector.js
index 8380c1b0e..c4bb79821 100644
--- a/app/assets/javascripts/language_selector.js
+++ b/app/assets/javascripts/language_selector.js
@@ -12,3 +12,7 @@ $(document).on("click", "#select_language_dialog [data-language-code]", function
     location.reload();
   }
 });
+
+$(document).on("submit", "#select_language_form", function (e) {
+  e.preventDefault();
+});
diff --git a/app/views/languages_panes/show.html.erb b/app/views/languages_panes/show.html.erb
index 38a5340f6..82bad7990 100644
--- a/app/views/languages_panes/show.html.erb
+++ b/app/views/languages_panes/show.html.erb
@@ -1,6 +1,6 @@
 <%= turbo_frame_tag "select_language_list" do %>
   <% if current_user&.id %>
-    <%= form_tag basic_preferences_path, :method => "PUT" do %>
+    <%= form_tag basic_preferences_path, :method => "PUT", :id => "select_language_form", "data-turbo" => false do %>
       <%= hidden_field_tag "referer", @source_page %>
       <%= hidden_field_tag "language", I18n.locale %>
       <%= render "select_language_list" %>

[tomhughes]

That certainly looks much nicer to me if it works, and targeting the submit event rather than keypresses was roughly what I had in mind as well.

[prvshkmr]

Checking for specific key presses can lead to other issues, accessibility being a common example.

The issue is with the form submitting at the wrong moment. Therefore it's probably the submission that should be tackled. There's an event for that.

I played a bit with this today. I can't tell this is 100% the best option, wondering what others think:

diff --git a/app/assets/javascripts/language_selector.js b/app/assets/javascripts/language_selector.js
index 8380c1b0e..c4bb79821 100644
--- a/app/assets/javascripts/language_selector.js
+++ b/app/assets/javascripts/language_selector.js
@@ -12,3 +12,7 @@ $(document).on("click", "#select_language_dialog [data-language-code]", function
     location.reload();
   }
 });
+
+$(document).on("submit", "#select_language_form", function (e) {
+  e.preventDefault();
+});
diff --git a/app/views/languages_panes/show.html.erb b/app/views/languages_panes/show.html.erb
index 38a5340f6..82bad7990 100644
--- a/app/views/languages_panes/show.html.erb
+++ b/app/views/languages_panes/show.html.erb
@@ -1,6 +1,6 @@
 <%= turbo_frame_tag "select_language_list" do %>
   <% if current_user&.id %>
-    <%= form_tag basic_preferences_path, :method => "PUT" do %>
+    <%= form_tag basic_preferences_path, :method => "PUT", :id => "select_language_form", "data-turbo" => false do %>
       <%= hidden_field_tag "referer", @source_page %>
       <%= hidden_field_tag "language", I18n.locale %>
       <%= render "select_language_list" %>

Yeah, this makes sense — handling the submit event directly is way cleaner than messing around with key checks, and it avoids the accessibility headaches.

[hlfan]

@pablobm would you mind making a pr with your patch?
I'd like to have the metadata in more than just a co-authored by.