Add form field id generation, remove duplicated ids (#30546) (#30561)

Backport #30546 by @silverwind

Fixes: https://github.com/go-gitea/gitea/issues/30384
On repo settings page, there id `repo_name` was used 5 times on the same
page, some in modal and such. I think we are better off just
auto-generating these IDs in the future so that labels link up with
their form element.

Ideally this id generation would be done in backend in a subtemplate,
but seeing that we already have similar JS patches for checkboxes, I
took the easy path for now.

I also checked that these `#repo_name` were not in use in JS and the
only case where this id appears in JS is on the migration page where
it's still there.

Co-authored-by: silverwind <me@silverwind.io>

diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl
index df6ccbf6bc2fbd5679ff53553858fa2cdafef029..f1b85bfaa99dc7e4596b6790a4e8351bcfaa47c3 100644 (file)
--- a/templates/repo/settings/options.tmpl
+++ b/templates/repo/settings/options.tmpl
@@ -9,8 +9,8 @@
                                {{.CsrfTokenHtml}}
                                <input type="hidden" name="action" value="update">
                                <div class="required field {{if .Err_RepoName}}error{{end}}">
-                                       <label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
-                                       <input id="repo_name" name="repo_name" value="{{.Repository.Name}}" data-repo-name="{{.Repository.Name}}" autofocus required>
+                                       <label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
+                                       <input name="repo_name" value="{{.Repository.Name}}" data-repo-name="{{.Repository.Name}}" autofocus required>
                                </div>
                                <div class="inline field">
                                        <label>{{ctx.Locale.Tr "repo.repo_size"}}</label>
@@ -884,8 +884,8 @@
                                                </label>
                                        </div>
                                        <div class="required field">
-                                               <label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
-                                               <input id="repo_name" name="repo_name" required maxlength="100">
+                                               <label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
+                                               <input name="repo_name" required maxlength="100">
                                        </div>
 
                                        <div class="text right actions">
@@ -915,8 +915,8 @@
                                                </label>
                                        </div>
                                        <div class="required field">
-                                               <label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
-                                               <input id="repo_name" name="repo_name" required>
+                                               <label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
+                                               <input name="repo_name" required>
                                        </div>
 
                                        <div class="text right actions">
@@ -947,8 +947,8 @@
                                        </label>
                                </div>
                                <div class="required field">
-                                       <label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
-                                       <input id="repo_name" name="repo_name" required>
+                                       <label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
+                                       <input name="repo_name" required>
                                </div>
                                <div class="required field">
                                        <label for="new_owner_name">{{ctx.Locale.Tr "repo.settings.transfer_owner"}}</label>
@@ -1017,8 +1017,8 @@
                                        </label>
                                </div>
                                <div class="required field">
-                                       <label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
-                                       <input id="repo_name" name="repo_name" required>
+                                       <label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
+                                       <input name="repo_name" required>
                                </div>
 
                                <div class="text right actions">

diff --git a/web_src/js/modules/fomantic.js b/web_src/js/modules/fomantic.js
index d205c2b2ee57d85637d56c665b1e9d0d27cdcf9f..c04bc6e8636e0278b0ad30856046caf8c53ad8b0 100644 (file)
--- a/web_src/js/modules/fomantic.js
+++ b/web_src/js/modules/fomantic.js
@@ -1,6 +1,7 @@
 import $ from 'jquery';
 import {initFomanticApiPatch} from './fomantic/api.js';
 import {initAriaCheckboxPatch} from './fomantic/checkbox.js';
+import {initAriaFormFieldPatch} from './fomantic/form.js';
 import {initAriaDropdownPatch} from './fomantic/dropdown.js';
 import {initAriaModalPatch} from './fomantic/modal.js';
 import {initFomanticTransition} from './fomantic/transition.js';
@@ -27,6 +28,7 @@ export function initGiteaFomantic() {
 
   // Use the patches to improve accessibility, these patches are designed to be as independent as possible, make it easy to modify or remove in the future.
   initAriaCheckboxPatch();
+  initAriaFormFieldPatch();
   initAriaDropdownPatch();
   initAriaModalPatch();
 }

diff --git a/web_src/js/modules/fomantic/base.js b/web_src/js/modules/fomantic/base.js
index c4a01038ba3e0a48829bb8674fe98815146e58ef..7574fdd25cce7220732d2984aa326662120430b3 100644 (file)
--- a/web_src/js/modules/fomantic/base.js
+++ b/web_src/js/modules/fomantic/base.js
@@ -3,3 +3,16 @@ let ariaIdCounter = 0;
 export function generateAriaId() {
   return `_aria_auto_id_${ariaIdCounter++}`;
 }
+
+export function linkLabelAndInput(label, input) {
+  const labelFor = label.getAttribute('for');
+  const inputId = input.getAttribute('id');
+
+  if (inputId && !labelFor) { // missing "for"
+    label.setAttribute('for', inputId);
+  } else if (!inputId && !labelFor) { // missing both "id" and "for"
+    const id = generateAriaId();
+    input.setAttribute('id', id);
+    label.setAttribute('for', id);
+  }
+}

diff --git a/web_src/js/modules/fomantic/checkbox.js b/web_src/js/modules/fomantic/checkbox.js
index 7f2b340296994d5f84406af0d5e719503ca556af..ed77406cc397ceae6477db8ef26878126aa3060a 100644 (file)
--- a/web_src/js/modules/fomantic/checkbox.js
+++ b/web_src/js/modules/fomantic/checkbox.js
@@ -1,4 +1,4 @@
-import {generateAriaId} from './base.js';
+import {linkLabelAndInput} from './base.js';
 
 export function initAriaCheckboxPatch() {
   // link the label and the input element so it's clickable and accessible
@@ -7,18 +7,7 @@ export function initAriaCheckboxPatch() {
     const label = el.querySelector('label');
     const input = el.querySelector('input');
     if (!label || !input) continue;
-    const inputId = input.getAttribute('id');
-    const labelFor = label.getAttribute('for');
-
-    if (inputId && !labelFor) { // missing "for"
-      label.setAttribute('for', inputId);
-    } else if (!inputId && !labelFor) { // missing both "id" and "for"
-      const id = generateAriaId();
-      input.setAttribute('id', id);
-      label.setAttribute('for', id);
-    } else {
-      continue;
-    }
+    linkLabelAndInput(label, input);
     el.setAttribute('data-checkbox-patched', 'true');
   }
 }

diff --git a/web_src/js/modules/fomantic/form.js b/web_src/js/modules/fomantic/form.js
new file mode 100644 (file)
index 0000000..3bb0058
--- /dev/null
+++ b/web_src/js/modules/fomantic/form.js
@@ -0,0 +1,13 @@
+import {linkLabelAndInput} from './base.js';
+
+export function initAriaFormFieldPatch() {
+  // link the label and the input element so it's clickable and accessible
+  for (const el of document.querySelectorAll('.ui.form .field')) {
+    if (el.hasAttribute('data-field-patched')) continue;
+    const label = el.querySelector(':scope > label');
+    const input = el.querySelector(':scope > input');
+    if (!label || !input) continue;
+    linkLabelAndInput(label, input);
+    el.setAttribute('data-field-patched', 'true');
+  }
+}