web: the topics-remove field gets a label, and a test that would have caught it !254
4 files changed, +89 −3
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
| @@ -19,9 +19,10 @@ | ||
| 19 | 19 | </form> |
| 20 | 20 | <form method="post" action="{{$base}}" class="setform"> |
| 21 | 21 | <input type="hidden" name="field" value="topics"> |
| 22 | <label for="topics-add">Topics</label> | |
| 22 | <label for="topics-add">Topics to add</label> | |
| 23 | 23 | <input type="text" id="topics-add" name="add" placeholder="add, space-separated"> |
| 24 | <input type="text" name="remove" placeholder="remove"> | |
| 24 | <label for="topics-remove">Topics to remove</label> | |
| 25 | <input type="text" id="topics-remove" name="remove" placeholder="remove"> | |
| 25 | 26 | <button type="submit">Apply</button> |
| 26 | 27 | </form> |
| 27 | 28 | {{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 | 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 | 261 | // Pages lists the page template names, for tests that render each one. |
| 253 | 262 | func Pages() []string { |
| 254 | 263 | names := make([]string, 0, len(pages)) |
sonar-project.properties +7 −1
| @@ -12,7 +12,13 @@ sonar.test.inclusions=**/*_test.go | ||
| 12 | 12 | |
| 13 | 13 | # dist/ is release output, testdata is fixtures meant to be malformed, and |
| 14 | 14 | # the fonts are third-party binaries. |
| 15 | sonar.exclusions=dist/**,**/testdata/**,internal/web/static/fonts/** | |
| 15 | # | |
| 16 | # The migrations are excluded because they are SQLite and the analyser | |
| 17 | # reads .sql as PL/SQL, where '' is NULL. That turns `WHERE col = ''` on a | |
| 18 | # NOT NULL DEFAULT '' column — correct SQLite, and the shape used | |
| 19 | # throughout — into a NullComparison finding. Excluding them is the fix; | |
| 20 | # dismissing the same false positive after every migration is not. | |
| 21 | sonar.exclusions=dist/**,**/testdata/**,internal/web/static/fonts/**,internal/store/migrations/** | |
| 16 | 22 | |
| 17 | 23 | # No sonar.go.coverage.reportPaths yet. Most of this repository's coverage |
| 18 | 24 | # comes from the e2e suite, and re-running that under -coverprofile would |