From 6d7117a361b86f4c8f03a75a8aaca2b90b16143b Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 00:11:48 +0200 Subject: [PATCH] Never guess a label from another control's, form's or link's text; test the radio group with nothing checked (issue 1530) --- modules/brother/cmd/brother/html.go | 15 ++++-- modules/brother/cmd/brother/html_test.go | 49 +++++++++++++++++-- modules/brother/cmd/brother/printer_test.go | 10 ++-- .../admin/reconstructed-scanfrompc.html | 3 +- 4 files changed, 62 insertions(+), 15 deletions(-) diff --git a/modules/brother/cmd/brother/html.go b/modules/brother/cmd/brother/html.go index 60b598c6..58a16eb0 100644 --- a/modules/brother/cmd/brother/html.go +++ b/modules/brother/cmd/brother/html.go @@ -162,6 +162,7 @@ func ParsePage(body []byte) Page { if (cellOpen && cellN == 1) || f.row == "" { f.row = lastText } + lastText = "" // text before a control names that control alone form.Fields = append(form.Fields, f) if inLabel && labelFor == "" { wrapped = append(wrapped, len(form.Fields)-1) @@ -176,11 +177,16 @@ func ParsePage(body []byte) Page { case inTextA: textA.WriteString(t) } - if !inTitle && !inOption && !inTextA { + if !inTitle && !inOption && !inTextA && link == nil { if q := squash(t); q != "" { - lastText = q - if tailIdx >= 0 && form.Fields[tailIdx].own == "" { - form.Fields[tailIdx].own = q + switch { + case tailIdx >= 0: + // the text right after a radio button is that button's, not the next field's + if form.Fields[tailIdx].own == "" { + form.Fields[tailIdx].own = q + } + case !inLabel: + lastText = q } } if cellOpen && cellN == 1 { @@ -282,6 +288,7 @@ func ParsePage(body []byte) Page { } case "form": closeForm() + lastText = "" if !closing { formOpen = true form = Form{Action: a["action"], Method: strings.ToLower(a["method"])} diff --git a/modules/brother/cmd/brother/html_test.go b/modules/brother/cmd/brother/html_test.go index a54a1697..2df0abdc 100644 --- a/modules/brother/cmd/brother/html_test.go +++ b/modules/brother/cmd/brother/html_test.go @@ -74,11 +74,50 @@ func TestAFieldIsLabelledByItsTableRow(t *testing.T) { } } -// 1530: the reconstructed scan-from-PC page answers its radio group whole. -func TestTheScanFromPCPageAnswersItsRadioGroup(t *testing.T) { +// 1530: the scan-from-PC page, RECONSTRUCTED (its checked button and row heading are assumptions, only the form +// actions, field names, option labels and values come from the live answer): only its structure is asserted. +func TestTheReconstructedScanFromPCPageHasItsRadioGroupAsOneField(t *testing.T) { p := ParsePage([]byte(adminFixture(t, "reconstructed-scanfrompc.html"))) - f := fieldNamed(t, p, "B117d") - if f.Value != "1" || f.Label != "Scan from PC" || len(f.Options) != 2 { - t.Errorf("B117d = %+v", f) + if len(p.Forms) != 2 { + t.Fatalf("forms = %+v", p.Forms) + } + var radios []Field + for _, f := range p.Forms[1].Fields { + if f.Type == "radio" { + radios = append(radios, f) + } + } + if len(radios) != 1 || radios[0].Name != "B117d" || len(radios[0].Options) != 2 || + radios[0].Options[0] != (Option{Value: "1", Label: "Enabled"}) || radios[0].Options[1] != (Option{Value: "0", Label: "Disabled"}) { + t.Errorf("radios = %+v", radios) + } + first := p.Forms[0] + if first.Action != "/home/status.html" || len(first.Fields) != 2 || first.Fields[0].Name != "CSRFToken" || first.Fields[1].Name != "B1892" { + t.Errorf("first form = %+v", first) + } +} + +// 1530: a radio group with no checked button answers value "" and both options. +func TestARadioGroupWithNothingCheckedAnswersNoValue(t *testing.T) { + p := ParsePage([]byte(`
`)) + f := fieldNamed(t, p, "R") + if f.Value != "" || len(f.Options) != 2 { + t.Errorf("group = %+v", f) + } +} + +// 1530: a label is never guessed from text that is not the field's: the text after a radio button, the text of +// another form, a menu link, a layout table's menu. The label is empty instead. +func TestAFieldIsNotLabelledByOtherTexts(t *testing.T) { + for name, c := range map[string]struct{ html, field string }{ + "after a radio group": {`
Off
`, "X"}, + "after a label": {`
`, "X"}, + "in a second form": {`
Disabled
`, "X"}, + "after a menu link": {`Scan from PC
`, "X"}, + "layout table menu": {`
MenuStatus
`, "X"}, + } { + if got := fieldNamed(t, ParsePage([]byte(c.html)), c.field).Label; got != "" { + t.Errorf("%s: label = %q, want none", name, got) + } } } diff --git a/modules/brother/cmd/brother/printer_test.go b/modules/brother/cmd/brother/printer_test.go index dfa8cff8..4e841d2b 100644 --- a/modules/brother/cmd/brother/printer_test.go +++ b/modules/brother/cmd/brother/printer_test.go @@ -45,10 +45,10 @@ func TestPrinterSettingsReadsAPageBehindTheLogin(t *testing.T) { t.Fatalf("forms = %+v", a.Forms) } want := map[string]Field{ - "lock": {Name: "lock", Type: "checkbox", Value: "on", Label: "Scan lock"}, - "node": {Name: "node", Type: "text", Value: "BRN-SYNTH", Label: "Name"}, - "pw": {Name: "pw", Type: "password", Value: "", Label: "Secret"}, - "note": {Name: "note", Type: "textarea", Value: "hello & bye"}, + "lock": {Name: "lock", Type: "checkbox", Value: "on", Label: "Scan lock"}, + "node": {Name: "node", Type: "text", Value: "BRN-SYNTH", Label: "Name"}, + "pw": {Name: "pw", Type: "password", Value: "", Label: "Secret"}, + "note": {Name: "note", Type: "textarea", Value: "hello & bye"}, } for _, fl := range a.Forms[0].Fields { if fl.Name == "mode" { @@ -276,7 +276,7 @@ func TestPrinterSettingsLeavesOutLinksAndHiddenFieldsUnlessAsked(t *testing.T) { if l, _ := more["links"].([]any); len(l) != 3 { t.Errorf("links:true gave %v", more["links"]) } - if names(more) != "pageid,CSRFToken,postif_registration_reject,B117d" { + if names(more) != "CSRFToken,B1892,pageid,CSRFToken,postif_registration_reject,B117d" { t.Errorf("hidden:true fields = %s", names(more)) } } diff --git a/modules/brother/cmd/brother/testdata/admin/reconstructed-scanfrompc.html b/modules/brother/cmd/brother/testdata/admin/reconstructed-scanfrompc.html index 8efccbd0..68b3b9ca 100644 --- a/modules/brother/cmd/brother/testdata/admin/reconstructed-scanfrompc.html +++ b/modules/brother/cmd/brother/testdata/admin/reconstructed-scanfrompc.html @@ -1,8 +1,9 @@ Brother MFC-L8390CDW series StatusScan from PCLogout +
Scan from PC