Repository navigation
fix(cmux-next): a density change restyles the omnibar field in place - #17667
azooz2003-bit wants to merge 4 commits into
Conversation
…arked text Red on the current code: the density binding writes the field text again, which replaces the active field editor's storage, drops the marked text and puts the caret at the end. OmnibarViewTests changes the density while the other omnibar suites run, which is the flake in OmnibarFieldEditorTests and OmnibarSelectionViewTests (#17601, #17626). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The density binding wrote the field text again, which replaced the active field editor's storage: an input method's composition was dropped and the caret moved to the end of the URL. It now sets the font on the editor's text in place, as a theme change recolors it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…al change Changing DesignSettings.shared mid-test would disturb the suites running beside it, the failure this PR fixes. The test runs the bar's density appliers directly instead; restyle documents that it sets only the font while editing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Not verified: macOS jobs did not run: compile and app tests were skipped on CI passes on Written by |
2 participants
Summary
This is the root cause of the omnibar field-editor flakes:
OmnibarFieldEditorTests.markedTextNeverCompletesAndItsCommitDoes(3 of 27 failed jobs in a 60-run census, and #17626's 1 in 4 package runs) andOmnibarSelectionViewTests.aFocusingDragSelectsTheDraggedURL(#17601).AddressBarView'sDensityBindingobservesDesignSettings.shared, and on any change it calledfield.write(fieldText, ...). With the field editor active, that write replaces the editor's whole text storage. The input method's marked text is dropped, and the caret jumps to the end of the URL. Every failure matched that:"https://example.org/startgi", a{27, 0}caret, and{25, 0}in place of the dragged range.Ten test files change
DesignSettings.shared. One of them isOmnibarViewTests, in the same target, which sets the density while the other omnibar suites run in parallel. That's the cross-suite disturbance #17626 saw ("the field editor lost its IME marked text mid-test") without a named source.It's a product bug too: changing density in Settings while typing or composing in the address bar loses the composition and the selection.
The fix:
AddressField.restylesets the font on the active editor's text in place, the same wayviewDidChangeEffectiveAppearancealready recolors it. An inactive field is written again as before.With this fix, the isolation in #17626 and #17650 should no longer be needed. They're harmless to land as well.
Testing
aDensityChangeKeepsTheSelectionAndMarkedTextwas committed first, red: focus selects the URL, then the density changes, and the selection must survive; then it composesgi, changes the density, and expects the marked text and caret kept. The old code fails it on any machine.cmux-next swift testjob is the green check.scripts/cmux-next/check-concurrency.shpasses.Changelog
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Changing interface density while the address bar field editor is active no longer drops input-method composition or moves the caret to the end of the URL. The density binding used to rewrite the field text, replacing the editor's storage; it now sets the font on the active editor's text in place (the same way a theme change recolors), preserving the selection and marked text. Inactive fields are still rewritten as before. The regression test runs the bar's density appliers directly, so it never changes the app-wide density and can't disturb the omnibar suites running in parallel.
Written for commit ce1b7e6. Summary will update on new commits.