Repository navigation
Keep the right radios checked when forms move, leave or are renamed - #185
Merged
Merged
Conversation
… moves or is removed Chromium and Firefox briefly reset the form of a radio with a form attribute while a form around it moves or leaves, so a checked one unchecked a radio in another group. Those radios now move or leave unchecked and are checked again straight after. Fixes #162 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SZddej5CrkTfW4RWeFsvyU
… group Without preserveChanges, a checked radio that a form id change moved into another group was checked again when the morph settled, unchecking a later radio the markup checks there. Its group is now synced, so the last checked radio wins as when parsing. Fixes #180 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SZddej5CrkTfW4RWeFsvyU
joeldrapper
marked this pull request as ready for review
October 7, 2026 12:55
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SZddej5CrkTfW4RWeFsvyU
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6356c72af2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e forms Radios with a form attribute that are unchecked for a move and checked again straight after now go through the attribute, so an untouched one keeps following it without preserveChanges. Inserting a live target that holds a form protects those radios too, even when the target's markup checks nothing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SZddej5CrkTfW4RWeFsvyU
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.
Requested by Joel · project thread
Fixes #162. Fixes #180.
Before: when a morph moved a form holding a checked radio with a
formattribute, the radio briefly joined another group and unchecked the radio there. That could be the user's pick in the moved form (underpreserveChanges), or a formless radio outside it (in both modes, and in Firefox as well as Chromium). Removing such a form did the same (#162). Separately, withoutpreserveChanges, a user-checked radio that a form id change moved into another group was checked again when the morph settled, which unchecked a later radio that the markup checks in that group (#180).After: those radios stay checked, and the last radio the markup checks in a group wins, as when parsing.
How:
formattribute while a form around it moves or leaves. So when a node holding a form moves (placing children or completing a cross-parent move) or is removed, the checked radios inside it that have aformattribute now go unchecked first. They are checked again straight after, back in the group they were in. This step only runs when the node holds a form, so moving ordinary rows costs the same as before.Tests:
test/new/moved-form-radios.browser.test.tscovers the cases from #162 and its comments, including the Firefox default-mode seed 37617970929. It also covers removing a form, which the form-state fuzzer from #167 found on main (seed 9104741). All six fail on main in Chromium, and four of them fail in Firefox.test/new/renamed-form-radios.browser.test.tscovers #180 and fails on main. With the fix, 24,000 form-state fuzzer seeds from #167 pass in Chromium and Firefox, apart from one seed (9303793) that also fails on main. That seed involves a select's selection, not radios. The full suite passes at 100% coverage.Benchmark (perf-sweep harness, 31 rounds, lower quartile vs main): every scenario is within noise of main, from −6% to +3% (keyed reverse +3%, shuffle +2%). The #180 change only runs at settle, for radios that were held back.
🤖 Generated with Claude Code
https://claude.ai/code/session_01SZddej5CrkTfW4RWeFsvyU