Certificates page: make the netboot transport two radios, titled as the force it performs - #1731
Closed
darksidemilk wants to merge 2 commits into
Closed
darksidemilk wants to merge 2 commits into
darksidemilk wants to merge 2 commits into
Conversation
…erforms Two problems in one row, both in what the browser receives. It rendered as a `form-select-sm` in the table's narrow first column, which came out as a bare `http` chip that does not look interactive -- it reads as a status badge beside three switches that are obviously controls. A select also shows one value and hides the other behind a click, on a row whose entire subject is a choice between two transports. Two radios show the domain AND the current value at once, which is what the switches next to them do. And it was titled "Netboot fetches boot.php over", a dangling phrase the select was there to complete. With the control in the cell to its left the sentence no longer finishes, and the three rows above it are all complete statements. Titled "Force netboot over HTTP or HTTPS", because forcing is what writing this key does and the old title hid it. The page cannot set BOOT_url_proto_forced -- it is deliberately absent from the helper's allowlist, and ADR 0036 spends a rejected alternative on why -- but it does not need to: _resolveInstallMode() sets that key to yes whenever an explicit value is supplied, so recording a transport here IS forcing it. The row's description now names the key that records it and says the derivation stops, rather than only mentioning it for the https case. The title's label points at the radio that is ALREADY checked, so the setting name stays the click target the switches' names are without a click on it changing anything. Pointing it at a fixed id would have made one click on the title silently force the transport, which is the misclick that same rejected alternative refused to expose. JS follows the control. `change` fires only on the radio being selected and never on the one being cleared, so it still posts once per click. The revert on error re-checks a sibling rather than restoring a value, and the group is found by shared `name` -- the attribute that makes them mutually exclusive in the first place, so it cannot drift from the group the browser sees. tests/certificate-table.test.php pins the rendered row: two radios and no select, one shared name, exactly one checked and it is the reported transport, the force wording, the key named, the already-checked `for` target, and both radios disabled without system.pki. Nine of the ten fail against the previous markup, and dropping the shared `name` fails exactly the assertion about it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ps28eGTALgieR6TBUSafVg
Member
Author
|
Superseded by #1732. Radios were the wrong control here: the question this row asks is not which transport but forced to https, or left to derive — and while building it I found the actual bug, which #1731 would have shipped intact. Both |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before
Two problems, both in what the browser receives.
It does not look like a control. A
form-select-smin the table's narrowfirst column renders as a bare
httpchip — a status badge sitting beside threeswitches that are obviously interactive. A select also shows one value and hides
the other behind a click, on a row whose entire subject is a choice between two
transports.
The title is a dangling phrase.
Netboot fetches boot.php overwas writtento be completed by the select. With the control in the cell to its left the
sentence never finishes, and the three rows above it are all complete
statements.
After
Two radios show the domain and the current value at once, which is what the
switches beside them do. This does not fight the prior decision: the comment
that chose a select argued "its domain is http|https, so it is a select rather
than a switch, and a checkbox would have to invent which way is 'on'" — radios
satisfy that reasoning better than the select did.
Titled
Force, because forcing is what writing this key does. The pagecannot set
BOOT_url_proto_forced— it is deliberately absent from the helper'sallowlist, and ADR 0036 spends a rejected alternative on why — but it does not
need to:
_resolveInstallMode()sets that key toyeswhenever an explicitvalue is supplied, so recording a transport here is forcing it. The old
title hid that. The description now names the key that records it and says the
derivation stops, rather than mentioning it only for the
httpscase.The title's label points at the radio that is already checked, so the
setting name stays the click target the switches' names are without a click on
it changing anything. Pointing it at a fixed id would make one click on the
title silently force the transport — the exact misclick that same rejected
alternative refused to expose.
JS
changefires only on the radio being selected, never on the one beingcleared, so it still posts once per click and keeps the one-key-per-call shape.
The revert on error re-checks a sibling rather than restoring a value, and the
group is located by shared
name— the attribute that makes them mutuallyexclusive in the first place, so it cannot drift from the group the browser
sees.
Testing
tests/certificate-table.test.phpgains ten assertions on the rendered row,not on a call site — the failure mode here is entirely in the markup. A radio
group that lost its shared
namestill renders as two radios and still posts,but both can be checked at once and the row stops describing one setting.
nameremovedThe tenth is a negative guard (the writable render disables neither radio) and
passes either way by design.
tests/certificate-management-permission.test.php19/19 — it inspects thehelper's allowlist, which this does not touch, and is the test that would catch
an attempt to make
BOOT_url_proto_forcedsettable from the page.Not verified locally
phpstan(both passes) was not run — novendor/on this machine. Worthnoting because the
tests/pass baselines occurrence counts, and this addsa
preg_match_alland two closures to a baselined file; if acount:needsbumping, CI is where it will show.
xgettexthere, and thepre-commit hook skipped it. One translatable string changed and one was
reworded, so
messages.potand the.pofiles will drift until theregeneratejob corrects them.🤖 Generated with Claude Code
https://claude.ai/code/session_01Ps28eGTALgieR6TBUSafVg