From 4e8318d0ae5859f61fbc05ec0cc007cd00247eb2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?=
 <sascha.issbruecker@googlemail.com>
Date: Thu, 3 Jul 2025 08:44:41 +0200
Subject: [PATCH] Improve bookmark form accessibility (#1116)

* Bump Django

* Render error messages in English

* Remove unused USE_L10N option

* Associate errors and help texts with form fields

* Make checkbox inputs clickable

* Change cancel button text

* Fix tests
---
 bookmarks/forms.py                            |  9 ++-
 .../frontend/behaviors/tag-autocomplete.js    |  1 +
 .../components/TagAutocomplete.svelte         |  2 +
 bookmarks/settings/base.py                    |  2 -
 bookmarks/styles/theme/forms.css              | 13 ++--
 bookmarks/templates/bookmarks/form.html       | 37 +++++------
 bookmarks/templates/shared/error_list.html    |  6 ++
 bookmarks/templatetags/shared.py              | 27 ++++++++
 bookmarks/tests/test_bookmark_edit_view.py    | 33 +++++-----
 bookmarks/tests/test_bookmark_new_view.py     | 61 ++++++++++---------
 .../e2e_test_bookmark_details_modal.py        |  2 +-
 requirements.dev.txt                          |  4 +-
 requirements.txt                              |  4 +-
 13 files changed, 123 insertions(+), 78 deletions(-)
 create mode 100644 bookmarks/templates/shared/error_list.html

diff --git a/bookmarks/forms.py b/bookmarks/forms.py
index bdd15c0..5aa3efc 100644
--- a/bookmarks/forms.py
+++ b/bookmarks/forms.py
@@ -1,4 +1,5 @@
 from django import forms
+from django.forms.utils import ErrorList
 
 from bookmarks.models import Bookmark, build_tag_string
 from bookmarks.validators import BookmarkURLValidator
@@ -6,6 +7,10 @@ from bookmarks.type_defs import HttpRequest
 from bookmarks.services.bookmarks import create_bookmark, update_bookmark
 
 
+class CustomErrorList(ErrorList):
+    template_name = "shared/error_list.html"
+
+
 class BookmarkForm(forms.ModelForm):
     # Use URLField for URL
     url = forms.CharField(validators=[BookmarkURLValidator()])
@@ -48,7 +53,9 @@ class BookmarkForm(forms.ModelForm):
         if instance is not None and request.method == "GET":
             initial = {"tag_string": build_tag_string(instance.tag_names, " ")}
         data = request.POST if request.method == "POST" else None
-        super().__init__(data, instance=instance, initial=initial)
+        super().__init__(
+            data, instance=instance, initial=initial, error_class=CustomErrorList
+        )
 
     @property
     def is_auto_close(self):
diff --git a/bookmarks/frontend/behaviors/tag-autocomplete.js b/bookmarks/frontend/behaviors/tag-autocomplete.js
index 7221582..1e0c245 100644
--- a/bookmarks/frontend/behaviors/tag-autocomplete.js
+++ b/bookmarks/frontend/behaviors/tag-autocomplete.js
@@ -19,6 +19,7 @@ class TagAutocomplete extends Behavior {
         name: input.name,
         value: input.value,
         placeholder: input.getAttribute("placeholder") || "",
+        ariaDescribedBy: input.getAttribute("aria-describedby") || "",
         variant: input.getAttribute("variant"),
       },
     });
diff --git a/bookmarks/frontend/components/TagAutocomplete.svelte b/bookmarks/frontend/components/TagAutocomplete.svelte
index 8214eca..7a7b95d 100644
--- a/bookmarks/frontend/components/TagAutocomplete.svelte
+++ b/bookmarks/frontend/components/TagAutocomplete.svelte
@@ -6,6 +6,7 @@
   export let name;
   export let value;
   export let placeholder;
+  export let ariaDescribedBy;
   export let variant = 'default';
 
   let isFocus = false;
@@ -110,6 +111,7 @@
     <!-- autocomplete real input box -->
     <input id="{id}" name="{name}" value="{value ||''}" placeholder="{placeholder || ' '}"
            class="form-input" type="text" autocomplete="off" autocapitalize="off"
+           aria-describedby="{ariaDescribedBy}"
            on:input={handleInput} on:keydown={handleKeyDown}
            on:focus={handleFocus} on:blur={handleBlur}>
   </div>
diff --git a/bookmarks/settings/base.py b/bookmarks/settings/base.py
index 559ef8d..563337d 100644
--- a/bookmarks/settings/base.py
+++ b/bookmarks/settings/base.py
@@ -116,8 +116,6 @@ TIME_ZONE = os.getenv("TZ", "UTC")
 
 USE_I18N = True
 
-USE_L10N = True
-
 USE_TZ = True
 
 # Static files (CSS, JavaScript, Images)
diff --git a/bookmarks/styles/theme/forms.css b/bookmarks/styles/theme/forms.css
index 0a5b6ae..6be8773 100644
--- a/bookmarks/styles/theme/forms.css
+++ b/bookmarks/styles/theme/forms.css
@@ -224,12 +224,13 @@ textarea.form-input {
   position: relative;
 
   input {
-    clip: rect(0, 0, 0, 0);
-    height: 1px;
-    margin: -1px;
-    overflow: hidden;
+    opacity: 0;
     position: absolute;
-    width: 1px;
+    top: calc((var(--control-size-sm) - var(--control-icon-size)) / 2);
+    left: 0;
+    height: var(--control-icon-size);
+    width: var(--control-icon-size);
+    cursor: pointer;
 
     &:focus-visible + .form-icon {
       outline: var(--focus-outline);
@@ -243,9 +244,9 @@ textarea.form-input {
   }
 
   .form-icon {
+    pointer-events: none;
     border: var(--border-width) solid var(--checkbox-border-color);
     box-shadow: var(--input-box-shadow);
-    cursor: pointer;
     display: inline-block;
     position: absolute;
     transition:
diff --git a/bookmarks/templates/bookmarks/form.html b/bookmarks/templates/bookmarks/form.html
index 309dfc5..542ea59 100644
--- a/bookmarks/templates/bookmarks/form.html
+++ b/bookmarks/templates/bookmarks/form.html
@@ -1,5 +1,6 @@
 {% load widget_tweaks %}
 {% load static %}
+{% load shared %}
 
 <div class="bookmarks-form">
   {% csrf_token %}
@@ -7,7 +8,7 @@
   <div class="form-group {% if form.url.errors %}has-error{% endif %}">
     <label for="{{ form.url.id_for_label }}" class="form-label">URL</label>
     <div class="has-icon-right">
-      {{ form.url|add_class:"form-input"|attr:"autofocus"|attr:"placeholder: " }}
+      {{ form.url|form_field:"validation"|add_class:"form-input"|attr:"autofocus" }}
       <i class="form-icon loading"></i>
     </div>
     {% if form.url.errors %}
@@ -22,8 +23,8 @@
   </div>
   <div class="form-group" ld-tag-autocomplete>
     <label for="{{ form.tag_string.id_for_label }}" class="form-label">Tags</label>
-    {{ form.tag_string|add_class:"form-input"|attr:"autocomplete:off"|attr:"autocapitalize:off" }}
-    <div class="form-input-hint">
+    {{ form.tag_string|form_field:"help"|add_class:"form-input"|attr:"autocomplete:off"|attr:"autocapitalize:off" }}
+    <div id="{{ form.tag_string.auto_id }}_help" class="form-input-hint">
       Enter any number of tags separated by space and <strong>without</strong> the hash (#).
       If a tag does not exist it will be automatically created.
     </div>
@@ -35,7 +36,8 @@
       <label for="{{ form.title.id_for_label }}" class="form-label">Title</label>
       <div class="flex">
         <button id="refresh-button" class="btn btn-link suffix-button" type="button">Refresh from website</button>
-        <button ld-clear-button data-for="{{ form.title.id_for_label }}" class="ml-2 btn btn-link suffix-button clear-button"
+        <button ld-clear-button data-for="{{ form.title.id_for_label }}"
+                class="ml-2 btn btn-link suffix-button clear-button"
                 type="button">Clear
         </button>
       </div>
@@ -60,31 +62,31 @@
         <span class="form-label d-inline-block">Notes</span>
       </summary>
       <label for="{{ form.notes.id_for_label }}" class="text-assistive">Notes</label>
-      {{ form.notes|add_class:"form-input"|attr:"rows:8" }}
-      <div class="form-input-hint">
+      {{ form.notes|form_field:"help"|add_class:"form-input"|attr:"rows:8" }}
+      <div id="{{ form.notes.auto_id }}_help" class="form-input-hint">
         Additional notes, supports Markdown.
       </div>
     </details>
     {{ form.notes.errors }}
   </div>
   <div class="form-group">
-    <label for="{{ form.unread.id_for_label }}" class="form-checkbox">
-      {{ form.unread }}
+    <div class="form-checkbox">
+      {{ form.unread|form_field:"help" }}
       <i class="form-icon"></i>
-      <span>Mark as unread</span>
-    </label>
-    <div class="form-input-hint">
+      <label for="{{ form.unread.id_for_label }}">Mark as unread</label>
+    </div>
+    <div id="{{ form.unread.auto_id }}_help" class="form-input-hint">
       Unread bookmarks can be filtered for, and marked as read after you had a chance to look at them.
     </div>
   </div>
   {% if request.user_profile.enable_sharing %}
     <div class="form-group">
-      <label for="{{ form.shared.id_for_label }}" class="form-checkbox">
-        {{ form.shared }}
+      <div class="form-checkbox">
+        {{ form.shared|form_field:"help" }}
         <i class="form-icon"></i>
-        <span>Share</span>
-      </label>
-      <div class="form-input-hint">
+        <label for="{{ form.shared.id_for_label }}">Share</label>
+      </div>
+      <div id="{{ form.shared.auto_id }}_help" class="form-input-hint">
         {% if request.user_profile.enable_public_sharing %}
           Share this bookmark with other registered users and anonymous users.
         {% else %}
@@ -100,7 +102,7 @@
     {% else %}
       <input type="submit" value="Save" class="btn btn-primary btn btn-primary btn-wide">
     {% endif %}
-    <a href="{{ return_url }}" class="btn">Nevermind</a>
+    <a href="{{ return_url }}" class="btn">Cancel</a>
   </div>
   <script type="application/javascript">
     /**
@@ -227,6 +229,7 @@
             }
           });
       }
+
       refreshButton.addEventListener('click', refreshMetadata);
 
       // Fetch website metadata when page loads and when URL changes, unless we are editing an existing bookmark
diff --git a/bookmarks/templates/shared/error_list.html b/bookmarks/templates/shared/error_list.html
new file mode 100644
index 0000000..13e75ac
--- /dev/null
+++ b/bookmarks/templates/shared/error_list.html
@@ -0,0 +1,6 @@
+{% load i18n %}
+
+{# Force rendering validation errors in English language to align with the rest of the app #}
+{% language 'en-us' %}
+{% if errors %}<ul class="{{ error_class }}"{% if errors.field_id %} id="{{ errors.field_id }}_error"{% endif %}>{% for error in errors %}<li>{{ error }}</li>{% endfor %}</ul>{% endif %}
+{% endlanguage %}
\ No newline at end of file
diff --git a/bookmarks/templatetags/shared.py b/bookmarks/templatetags/shared.py
index 297c453..b067232 100644
--- a/bookmarks/templatetags/shared.py
+++ b/bookmarks/templatetags/shared.py
@@ -145,3 +145,30 @@ def render_markdown(context, markdown_text):
     linkified_html = bleach.linkify(sanitized_html)
 
     return mark_safe(linkified_html)
+
+
+def append_attr(widget, attr, value):
+    attrs = widget.attrs
+    if attrs.get(attr):
+        attrs[attr] += " " + value
+    else:
+        attrs[attr] = value
+
+
+@register.filter("form_field")
+def form_field(field, modifier_string):
+    modifiers = modifier_string.split(",")
+    has_errors = hasattr(field, "errors") and field.errors
+
+    if "validation" in modifiers and has_errors:
+        append_attr(field.field.widget, "aria-describedby", field.auto_id + "_error")
+    if "help" in modifiers:
+        append_attr(field.field.widget, "aria-describedby", field.auto_id + "_help")
+
+    # Some assistive technologies announce a field as invalid when it has the
+    # required attribute, even if the user has not interacted with the field
+    # yet. Set aria-invalid false to prevent this behavior.
+    if field.field.required and not has_errors:
+        append_attr(field.field.widget, "aria-invalid", "false")
+
+    return field
diff --git a/bookmarks/tests/test_bookmark_edit_view.py b/bookmarks/tests/test_bookmark_edit_view.py
index 0275442..bec1ee4 100644
--- a/bookmarks/tests/test_bookmark_edit_view.py
+++ b/bookmarks/tests/test_bookmark_edit_view.py
@@ -114,9 +114,8 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
 
         self.assertInHTML(
             f"""
-            <input type="text" name="url" value="{bookmark.url}" placeholder=" "
-                    autofocus class="form-input" required id="id_url">   
-        """,
+            <input type="text" name="url" aria-invalid="false" autofocus class="form-input" required id="id_url" value="{bookmark.url}">
+            """,
             html,
         )
 
@@ -124,7 +123,7 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
         self.assertInHTML(
             f"""
             <input type="text" name="tag_string" value="{tag_string}" 
-                    autocomplete="off" autocapitalize="off" class="form-input" id="id_tag_string">
+                    autocomplete="off" autocapitalize="off" class="form-input" id="id_tag_string" aria-describedby="id_tag_string_help">
         """,
             html,
         )
@@ -148,7 +147,7 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
 
         self.assertInHTML(
             f"""
-            <textarea name="notes" cols="40" rows="8" class="form-input" id="id_notes">
+            <textarea name="notes" cols="40" rows="8" class="form-input" id="id_notes" aria-describedby="id_notes_help">
                 {bookmark.notes}
             </textarea>
         """,
@@ -259,12 +258,12 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
 
         self.assertInHTML(
             """
-            <label for="id_shared" class="form-checkbox">
-              <input type="checkbox" name="shared" id="id_shared">
-              <i class="form-icon"></i>
-              <span>Share</span>
-            </label>            
-        """,
+            <div class="form-checkbox">
+                <input type="checkbox" name="shared" aria-describedby="id_shared_help" id="id_shared">
+                <i class="form-icon"></i>
+                <label for="id_shared">Share</label>
+            </div>
+            """,
             html,
             count=0,
         )
@@ -278,12 +277,12 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
 
         self.assertInHTML(
             """
-            <label for="id_shared" class="form-checkbox">
-              <input type="checkbox" name="shared" id="id_shared">
-              <i class="form-icon"></i>
-              <span>Share</span>
-            </label>            
-        """,
+            <div class="form-checkbox">
+                <input type="checkbox" name="shared" aria-describedby="id_shared_help" id="id_shared">
+                <i class="form-icon"></i>
+                <label for="id_shared">Share</label>
+            </div>
+            """,
             html,
             count=1,
         )
diff --git a/bookmarks/tests/test_bookmark_new_view.py b/bookmarks/tests/test_bookmark_new_view.py
index 5e73d83..c4eaca3 100644
--- a/bookmarks/tests/test_bookmark_new_view.py
+++ b/bookmarks/tests/test_bookmark_new_view.py
@@ -78,9 +78,9 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
         html = response.content.decode()
 
         self.assertInHTML(
-            '<input type="text" name="url" value="http://example.com" '
-            'placeholder=" " autofocus class="form-input" required '
-            'id="id_url">',
+            """
+            <input type="text" name="url" aria-invalid="false" autofocus class="form-input" required id="id_url" value="http://example.com">
+            """,
             html,
         )
 
@@ -117,9 +117,10 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
         html = response.content.decode()
 
         self.assertInHTML(
-            '<input type="text" name="tag_string" value="tag1 tag2 tag3" '
-            'class="form-input" autocomplete="off" autocapitalize="off" '
-            'id="id_tag_string">',
+            """
+            <input type="text" name="tag_string" value="tag1 tag2 tag3" 
+                aria-describedby="id_tag_string_help" autocapitalize="off" autocomplete="off" class="form-input" id="id_tag_string">
+            """,
             html,
         )
 
@@ -137,8 +138,8 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
                     <span class="form-label d-inline-block">Notes</span>
                 </summary>
                 <label for="id_notes" class="text-assistive">Notes</label>
-                <textarea name="notes" cols="40" rows="8" class="form-input" id="id_notes">**Find** more info [here](http://example.com)</textarea>
-                <div class="form-input-hint">
+                <textarea name="notes" cols="40" rows="8" class="form-input" id="id_notes" aria-describedby="id_notes_help">**Find** more info [here](http://example.com)</textarea>
+                <div id="id_notes_help" class="form-input-hint">
                     Additional notes, supports Markdown.
                 </div>
             </details>
@@ -196,12 +197,12 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
 
         self.assertInHTML(
             """
-            <label for="id_shared" class="form-checkbox">
-              <input type="checkbox" name="shared" id="id_shared">
-              <i class="form-icon"></i>
-              <span>Share</span>
-            </label>            
-        """,
+            <div class="form-checkbox">
+                <input type="checkbox" name="shared" aria-describedby="id_shared_help" id="id_shared">
+                <i class="form-icon"></i>
+                <label for="id_shared">Share</label>
+            </div>          
+            """,
             html,
             count=0,
         )
@@ -213,12 +214,12 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
 
         self.assertInHTML(
             """
-            <label for="id_shared" class="form-checkbox">
-              <input type="checkbox" name="shared" id="id_shared">
-              <i class="form-icon"></i>
-              <span>Share</span>
-            </label>            
-        """,
+            <div class="form-checkbox">
+                <input type="checkbox" name="shared" aria-describedby="id_shared_help" id="id_shared">
+                <i class="form-icon"></i>
+                <label for="id_shared">Share</label>
+            </div>              
+            """,
             html,
             count=1,
         )
@@ -231,10 +232,10 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
         html = response.content.decode()
         self.assertInHTML(
             """
-          <div class="form-input-hint">
-              Share this bookmark with other registered users.
-          </div>
-        """,
+              <div id="id_shared_help" class="form-input-hint">
+                  Share this bookmark with other registered users.
+              </div>
+            """,
             html,
         )
 
@@ -245,10 +246,10 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
         html = response.content.decode()
         self.assertInHTML(
             """
-          <div class="form-input-hint">
-              Share this bookmark with other registered users and anonymous users.
-          </div>
-        """,
+              <div id="id_shared_help" class="form-input-hint">
+                  Share this bookmark with other registered users and anonymous users.
+              </div>
+            """,
             html,
         )
 
@@ -265,7 +266,7 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
         html = response.content.decode()
 
         self.assertInHTML(
-            '<input type="checkbox" name="unread" id="id_unread">',
+            '<input type="checkbox" name="unread" id="id_unread" aria-describedby="id_unread_help">',
             html,
         )
 
@@ -277,6 +278,6 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
         html = response.content.decode()
 
         self.assertInHTML(
-            '<input type="checkbox" name="unread" id="id_unread" checked="">',
+            '<input type="checkbox" name="unread" id="id_unread" checked="" aria-describedby="id_unread_help">',
             html,
         )
diff --git a/bookmarks/tests_e2e/e2e_test_bookmark_details_modal.py b/bookmarks/tests_e2e/e2e_test_bookmark_details_modal.py
index c36e56f..4aee919 100644
--- a/bookmarks/tests_e2e/e2e_test_bookmark_details_modal.py
+++ b/bookmarks/tests_e2e/e2e_test_bookmark_details_modal.py
@@ -124,7 +124,7 @@ class BookmarkDetailsModalE2ETestCase(LinkdingE2ETestCase):
             # Cancel edit, verify return to details url
             details_url = url + f"&details={bookmark.id}"
             with self.page.expect_navigation(url=self.live_server_url + details_url):
-                self.page.get_by_text("Nevermind").click()
+                self.page.get_by_text("Cancel").click()
 
     def test_delete(self):
         bookmark = self.setup_bookmark()
diff --git a/requirements.dev.txt b/requirements.dev.txt
index 74f1c29..92aa7aa 100644
--- a/requirements.dev.txt
+++ b/requirements.dev.txt
@@ -12,9 +12,9 @@ click==8.1.7
     # via black
 coverage==7.6.1
     # via -r requirements.dev.in
-django==5.1.10
+django==5.2.3
     # via django-debug-toolbar
-django-debug-toolbar==4.4.6
+django-debug-toolbar==5.2.0
     # via -r requirements.dev.in
 execnet==2.1.1
     # via pytest-xdist
diff --git a/requirements.txt b/requirements.txt
index 9379244..f95aa0e 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -2,7 +2,7 @@
 # This file is autogenerated by pip-compile with Python 3.12
 # by the following command:
 #
-#    pip-compile requirements.in
+#    pip-compile
 #
 asgiref==3.8.1
     # via django
@@ -27,7 +27,7 @@ cryptography==43.0.1
     #   josepy
     #   mozilla-django-oidc
     #   pyopenssl
-django==5.1.10
+django==5.2.3
     # via
     #   -r requirements.in
     #   django-registration