Commit `5441d8b8bf`

5441d8b8bf33975b95b76721b48fb13d914126dc

parent: b0fba1c090

Verified · cmc ci/build: success ci/sonar: success ci/test: success ci/vuln: success

cmc <hello@cleberg.net> · 2026-09-04 23:15 UTC

web: the topics-remove field gets a label, and a test that would have caught it

A placeholder is not an accessible name: it disappears on focus, and a
screen reader is not required to announce it. The topics-remove input had
only a placeholder while the add field beside it had both an id and a
label — #133 fixed this class across the templates and missed this one.

TestEveryInputHasAnAccessibleName is the guard that was absent. It reads
the template source rather than rendered output, since the rule is a
property of the markup and holds whatever data a page is given.

It knows both associations. Six inputs use the implicit form,
`<label>Name <input></label>`, which is as good as for/id; a check that
knew only the explicit form reported all six, which is what the first
draft did before I looked at the markup rather than believing the count.

Migrations are excluded from analysis rather than dismissed again each
scan. They are SQLite and the analyser reads .sql as PL/SQL, where '' is
NULL, so `WHERE col = ''` on a NOT NULL DEFAULT '' column — correct
SQLite, and the shape used throughout — reads as a null-comparison bug.

Ref #153

Layout: unified · split

internal/httpd/inputlabels_test.go added +70

@@ -0,0 +1,70 @@
	1	package httpd
	2
	3	import (
	4	"regexp"
	5	"strings"
	6	"testing"
	7
	8	"gitbay.org/gitbay/internal/web"
	9	)
	10
	11	var (
	12	inputTag = regexp.MustCompile(`<input\b[^>]*>`)
	13	labelFor = regexp.MustCompile(`<label[^>]*\bfor="([^"]+)"`)
	14	labelWraps = regexp.MustCompile(`(?s)<label\b[^>]>.?</label>`)
	15	attrID = regexp.MustCompile(`\bid="([^"]+)"`)
	16	attrType = regexp.MustCompile(`\btype="([^"]+)"`)
	17	ariaLabel = regexp.MustCompile(`\baria-label(?:ledby)?="`)
	18	)
	19
	20	// Every input a person types into needs an accessible name: a <label for>
	21	// pointing at its id, or an aria-label. A placeholder is not one — it
	22	// disappears on focus and screen readers are not required to announce it.
	23	//
	24	// #133 fixed this class across the templates and missed the topics-remove
	25	// field, which the first SonarCloud scan then found
	26	// (Web:InputWithoutLabelCheck, #153). This is the guard that was absent.
	27	func TestEveryInputHasAnAccessibleName(t *testing.T) {
	28	// Types that carry their own name or take no input.
	29	exempt := map[string]bool{"hidden": true, "submit": true, "button": true, "reset": true, "image": true}
	30	for _, name := range web.Pages() {
	31	src, err := web.TemplateSource(name)
	32	if err != nil {
	33	t.Fatalf("%s: %v", name, err)
	34	}
	35	labelled := map[string]bool{}
	36	for _, m := range labelFor.FindAllStringSubmatch(src, -1) {
	37	labelled[m[1]] = true
	38	}
	39	// `<label>Name <input></label>` associates implicitly and is as
	40	// good as `for`/`id`. Six inputs use it, and a check that knew
	41	// only the explicit form would report every one of them.
	42	wrapped := labelWraps.FindAllStringIndex(src, -1)
	43	inWrappedLabel := func(at int) bool {
	44	for _, span := range wrapped {
	45	if at >= span[0] && at < span[1] {
	46	return true
	47	}
	48	}
	49	return false
	50	}
	51	for _, at := range inputTag.FindAllStringIndex(src, -1) {
	52	tag := src[at[0]:at[1]]
	53	typ := "text"
	54	if m := attrType.FindStringSubmatch(tag); m != nil {
	55	typ = m[1]
	56	}
	57	if exempt[typ] \|\| ariaLabel.MatchString(tag) \|\| inWrappedLabel(at[0]) {
	58	continue
	59	}
	60	m := attrID.FindStringSubmatch(tag)
	61	if m == nil {
	62	t.Errorf("%s: input has no id and is not inside a <label>: %s", name, strings.TrimSpace(tag))
	63	continue
	64	}
	65	if !labelled[m[1]] {
	66	t.Errorf("%s: input id=%q has no <label for>: %s", name, m[1], strings.TrimSpace(tag))
	67	}
	68	}
	69	}
	70	}

internal/web/templates/settings.html +3 −2

 </form>
 <form method="post" action="{{$base}}" class="setform">
   <input type="hidden" name="field" value="topics">
-  <label for="topics-add">Topics</label>
+  <label for="topics-add">Topics to add</label>
   <input type="text" id="topics-add" name="add" placeholder="add, space-separated">
-  <input type="text" name="remove" placeholder="remove">
+  <label for="topics-remove">Topics to remove</label>
+  <input type="text" id="topics-remove" name="remove" placeholder="remove">
   <button type="submit">Apply</button>
 </form>
 {{if .Topics}}<p class="meta">{{range .Topics}}<span class="chip topic">{{.}}</span> {{end}}</p>{{end}}

internal/web/web.go +9

	@@ -249,6 +249,15 @@ var pages = func() map[string]*template.Template {
249	return m	249	return m
250	}()	250	}()
251		251
		252	// TemplateSource returns a template's raw text, for tests that inspect the
		253	// markup rather than the rendered output — an accessibility rule about
		254	// labels and ids is a property of the source, and holds whatever data a
		255	// page is given.
		256	func TemplateSource(name string) (string, error) {
		257	b, err := templateFS.ReadFile("templates/" + name)
		258	return string(b), err
		259	}
		260
252	// Pages lists the page template names, for tests that render each one.	261	// Pages lists the page template names, for tests that render each one.
253	func Pages() []string {	262	func Pages() []string {
254	names := make([]string, 0, len(pages))	263	names := make([]string, 0, len(pages))

sonar-project.properties +7 −1

 # dist/ is release output, testdata is fixtures meant to be malformed, and
 # the fonts are third-party binaries.
-sonar.exclusions=dist/**,**/testdata/**,internal/web/static/fonts/**
+#
+# The migrations are excluded because they are SQLite and the analyser
+# reads .sql as PL/SQL, where '' is NULL. That turns `WHERE col = ''` on a
+# NOT NULL DEFAULT '' column — correct SQLite, and the shape used
+# throughout — into a NullComparison finding. Excluding them is the fix;
+# dismissing the same false positive after every migration is not.
+sonar.exclusions=dist/**,**/testdata/**,internal/web/static/fonts/**,internal/store/migrations/**
 # No sonar.go.coverage.reportPaths yet. Most of this repository's coverage
 # comes from the e2e suite, and re-running that under -coverprofile would

Commit 5441d8b8bf

Commit `5441d8b8bf`