fix(ui-macos): build text field cells through cellClass and pad them as CSS does (#11661)

[steinybot]

Fixes #11661

Problem

On macOS, a TextField with a custom font draws its idle text at a different height from its editing text, so the text moves on focus. The cause is the way setPadding was added to text fields and labels: Perry replaced the cell AppKit builds with a new inset cell. The new cell lost the factory setup, and the fixes that restored it led to #10155, #10856 and this issue.

Solution

Let AppKit build the inset cell, and make setPadding on a text field work as CSS padding does on the web: the text moves in by the padding, and the background fills the padding.

Screenshot

The first field is focused, so the field editor draws it. The other rows are idle.

Before	After

Changes

1. Build the inset cell through cellClass — textfield.rs
PerryTextField (textfield.rs) and PerrySecureTextField (securefield.rs) return the inset cells from cellClass. text_field and secure_text_field call textFieldWithString: on those classes.

   • Text and AttributedText use PerryLabel in text.rs. It takes only the inset cell, so a label keeps the line breaks in its value.
   • install_text_field_cell, install_secure_text_field_cell and install_label_cell are gone.
   • Each inset cell sets its Rust ivars in initTextCell:, because AppKit now allocates it.

   perry/crates/perry-ui-macos/src/widgets/textfield.rs

   Lines 205 to 214 in a743dd4

   	define_class!(
   	#[unsafe(super(NSTextField))]
   	#[name = "PerryTextField"]
   	pub struct PerryTextField;
   	impl PerryTextField {
   	#[unsafe(method(cellClass))]
   	fn cell_class() -> &'static AnyClass {
   	super::padding::PerryInsetTextFieldCell::class()
   	}

   View in diff →

2. Pad the frames AppKit hands the cell — padding.rs
   The cell pads drawInteriorWithFrame:inView: and editWithFrame:/selectWithFrame:, and no longer overrides drawingRectForBounds:. For a bezeled or bordered cell, AppKit's editing path runs the frame through drawingRectForBounds:. With the padding there, the editing text moved by the padding a second time.

   perry/crates/perry-ui-macos/src/widgets/padding.rs

   Lines 58 to 66 in a743dd4

   	// Padding goes on the frames AppKit hands in to draw and to edit, not
   	// in drawingRectForBounds:. AppKit's editing path for a borderless
   	// cell never calls drawingRectForBounds:, so padding there would put
   	// the editing text somewhere other than the idle text.
   	#[unsafe(method(drawInteriorWithFrame:inView:))]
   	fn draw_interior(&self, frame: CGRect, view: &NSView) {
   	let frame = inset_rect(frame, self.ivars().get(), view.isFlipped());
   	unsafe { msg_send![super(self), drawInteriorWithFrame: frame, inView: view] }
   	}

   View in diff →

3. Paint the text field background on its layer — textfield.rs
textfieldSetBackgroundColor uses the layer path of widgetSetBackgroundColor and turns the cell background off. The cell background filled only the area inside the padding.

   • Without a cell background, AppKit draws the text 2pt further left, idle and editing alike.

   perry/crates/perry-ui-macos/src/widgets/textfield.rs

   Lines 588 to 597 in a743dd4

   	pub fn set_background_color(handle: i64, r: f64, g: f64, b: f64, a: f64) {
   	if let Some(view) = super::get_widget(handle) {
   	// If the cell drew the background, it would fill only the text area
   	// inside the padding. The layer fills the whole field, as a CSS
   	// background does.
   	let tf: &NSTextField = unsafe { &*(Retained::as_ptr(&view) as *const NSTextField) };
   	tf.setDrawsBackground(false);
   	super::set_background_color(handle, r, g, b, a);
   	}
   	}

   View in diff →

4. Pass layer colours as a typed CGColor pointer — srgb.rs
   A c_void pointer failed objc2's message check in a debug build, so every layer background, border and shadow colour panicked there.

   perry/crates/perry-ui-macos/src/srgb.rs

   Lines 10 to 23 in a743dd4

   	/// CoreGraphics' opaque colour. A typed pointer encodes as `^{CGColor=}`, the
   	/// type CALayer's colour properties declare. If the pointer were `c_void`, it
   	/// would encode as `^v`, and a debug build's message check would reject the send.
   	#[repr(C)]
   	pub struct CGColor {
   	_private: [u8; 0],
   	}
   	unsafe impl objc2::encode::RefEncode for CGColor {
   	const ENCODING_REF: objc2::encode::Encoding =
   	objc2::encode::Encoding::Pointer(&objc2::encode::Encoding::Struct("CGColor", &[]));
   	}
   	type CGColorRef = *mut CGColor;

   View in diff →

5. Drop the manual setup in the widget builders — textfield.rs, securefield.rs, text.rs, attributed_text.rs
   The factory already makes the fields editable, bezeled and one line.

   • TextField no longer uses single-line mode, which placed the idle baseline on the system font for the control size.
   • SecureField uses textFieldWithString: in place of initWithFrame:, so a long value stays on one line and scrolls.

6. Keep the value to one line, as a web <input> does — textfield.rs
PerryTextField and PerrySecureTextField apply Chrome's rules for an input element, without single-line mode.

   • setStringValue: drops CR and LF, as input.value = … does.
   • Typed, pasted and dropped text turns each line break into one space.
   • Option-Return and Control-Return insert nothing. Return still submits.

   perry/crates/perry-ui-macos/src/widgets/textfield.rs

   Lines 636 to 656 in a743dd4

   	/// The characters that a web `<input>` treats as a line break.
   	const LINE_BREAKS: [char; 2] = ['\r', '\n'];
   	/// Removes every line break from `value`, as a web `<input>` does when code
   	/// sets its value. A TextField and a SecureField each hold one line.
   	pub(crate) fn strip_line_breaks(value: &NSString) -> Retained<NSString> {
   	let text = value.to_string();
   	if text.contains(LINE_BREAKS) {
   	NSString::from_str(&text.replace(LINE_BREAKS, ""))
   	} else {
   	value.retain()
   	}
   	}
   	/// Replaces each line break in typed, pasted or dropped text with one space,
   	/// as a web `<input>` does. `None` when `text` has no line break.
   	pub(crate) fn replace_line_breaks_with_spaces(text: &NSString) -> Option<Retained<NSString>> {
   	let text = text.to_string();
   	text.contains(LINE_BREAKS)
   	.then(|| NSString::from_str(&text.replace("\r\n", " ").replace(LINE_BREAKS, " ")))
   	}

   View in diff →

Verification

• cargo test -p perry-ui-macos passes, native_dynamic_colors included.
• native_textfield_cell checks TextField and SecureField, borderless, bezeled and bordered: a stable baseline on focus across five fonts, a long value on one line, padding by the exact amount, and a layer background. It fails on main.
• test-files/test_issue_11661_textfield_baseline.ts was compiled with a release build on macOS 26.6. The screenshots above come from that app, on main and on this branch.
• native_textfield_newlines puts line breaks into TextField and SecureField from code, typing, paste, drop and the newline commands, and checks the value, the height, and that Return submits.
• A padded borderless field with textfieldSetBackgroundColor was captured on screen idle and while editing. The text sat 3.5pt from the top and 1.5pt from the left in both.

Summary by CodeRabbit

• Bug Fixes
  • Fixed macOS text-field alignment, padding, and background rendering across border styles.
  • Text fields and secure fields now display as single-line inputs: inserted line breaks become spaces, while line breaks in regular text widgets are preserved.
  • Secure fields now scroll like regular text fields and retain consistent inset behavior.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 91dde398-d39c-42a3-a36c-c9cb6eb3c720

📥 Commits

Reviewing files that changed from the base of the PR and between 9e29f59 and 6d834c8.

📒 Files selected for processing (14)

• changelog.d/11665-macos-textfield-baseline.md
• crates/perry-ui-macos/Cargo.toml
• crates/perry-ui-macos/src/srgb.rs
• crates/perry-ui-macos/src/widgets/attributed_text.rs
• crates/perry-ui-macos/src/widgets/padding.rs
• crates/perry-ui-macos/src/widgets/securefield.rs
• crates/perry-ui-macos/src/widgets/text.rs
• crates/perry-ui-macos/src/widgets/textfield.rs
• crates/perry-ui-macos/tests/native_dynamic_colors.rs
• crates/perry-ui-macos/tests/native_text_color.rs
• crates/perry-ui-macos/tests/native_textfield_cell.rs
• crates/perry-ui-macos/tests/native_textfield_newlines.rs
• test-files/test_issue_11661_textfield_baseline.ts
• test-parity/known_failures.json

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The macOS text-field implementation now uses custom field subclasses and inset cells. It handles line breaks in programmatic values and user input, applies TextField backgrounds through the view layer, and adds native tests and a smoke fixture for rendering and input behavior.

Changes

macOS Text Fields

Layer / File(s)	Summary
Inset cell and label setup crates/perry-ui-macos/src/widgets/padding.rs, crates/perry-ui-macos/src/widgets/text.rs, crates/perry-ui-macos/src/widgets/attributed_text.rs, changelog.d/11665-macos-textfield-baseline.md, crates/perry-ui-macos/tests/native_text_color.rs	Inset cells now apply padding in drawInteriorWithFrame:inView:. PerryLabel uses the inset cell, and attributed text uses the shared label constructor.
Field construction and input behavior crates/perry-ui-macos/src/widgets/textfield.rs, crates/perry-ui-macos/src/widgets/securefield.rs, crates/perry-ui-macos/src/srgb.rs, crates/perry-ui-macos/tests/native_dynamic_colors.rs, changelog.d/11665-macos-textfield-baseline.md	Custom text and secure fields remove line breaks from programmatic values and replace line breaks in edits with spaces. TextField suppresses field-editor line-break commands and applies its background through the view layer. CGColorRef now uses an opaque CGColor type.
Rendering and input validation crates/perry-ui-macos/Cargo.toml, crates/perry-ui-macos/tests/native_textfield_cell.rs, crates/perry-ui-macos/tests/native_textfield_newlines.rs, test-files/test_issue_11661_textfield_baseline.ts, test-parity/known_failures.json	New macOS test targets check cell rendering, padding, backgrounds, newline handling, submission, and preservation of line breaks in Text widgets. A smoke fixture covers fields and labels; the parity entry records its Linux environment limitation.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 6d834

The changes align macOS field rendering and define single-line input behavior. No actionable merge-blocking issue was established; merging is reasonable after normal macOS checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6d834

The change affects how text and passwords are entered and displayed. Secure rendering remains delegated to the native password control, and no new disclosure or privilege-gain path was established. Editing recovery and credential lifetime are not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is application-local: supplied text and user editing reach native controls and registered application callbacks in the same process. The sensitive data includes password-field values. The inspected changes do not establish a new remote consumer, tenant boundary, or privileged service sink; broader security coverage remains incomplete.

Trust Boundaries and Controls

• observed — Secure-field notifications must match the registered field object's identity before its logical plaintext value is delivered to on_change. This application callback exposure exists in the available base comparison; the new subclass does not add a different recipient. Native password masking therefore remains distinct from application access to the value.

Resilience and Maintainability Implications

• observed — The inspected registry retains widget views, and field creation retains observers and callback entries. Container cleanup removes layout metadata without showing field-observer disposal. These ownership patterns predate the change in the available base comparison, providing counterevidence to a newly introduced dangling-pointer claim while leaving credential-retention and eventual-disposal guarantees unresolved.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The baseline and inset-cell changes relate to the historical context in [#11661]. The pull request also adds newline filtering and single-line command handling for TextField and SecureField, plus …	Remove the unrelated newline behavior and test, the CGColor FFI and dynamic-color changes, and the parity-tracking change from this pull request. Keep changes that implement the text-field baseline and inset-cell behavior, including relev…
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 11 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main macOS text-field fix: constructing cells through cellClass and applying CSS-like padding.
Description check	✅ Passed	The description explains the problem, solution, concrete changes, related issue, screenshots, and verification steps. It uses Problem, Solution, Changes, and Verification headings instead of the templ…
Linked Issues check	✅ Passed	Issue [#11661] is closed and supplies historical context only. No active directly linked issue provides coding requirements. Therefore, no linked-issue coding requirement applies to this pull request.

Full details: Out of Scope Changes check

Explanation

The baseline and inset-cell changes relate to the historical context in [#11661]. The pull request also adds newline filtering and single-line command handling for TextField and SecureField, plus native_textfield_newlines. It changes the CGColor FFI type, updates dynamic-color tests, and adds parity tracking. These changes are not connected to the baseline and inset-cell behavior described for [#11661].

Resolution

Remove the unrelated newline behavior and test, the CGColor FFI and dynamic-color changes, and the parity-tracking change from this pull request. Keep changes that implement the text-field baseline and inset-cell behavior, including relevant regression tests.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Copilot review overview

🔵 Needs a closer look

The unsafe Objective-C subclassing and AppKit rendering behavior require final macOS-based human validation.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes macOS text-field baseline shifts while preserving AppKit factory behavior and CSS-like padding.

Changes:

• Creates inset cells through cellClass and updates padding/background drawing.
• Enforces single-line TextField/SecureField newline behavior.
• Adds typed CGColor handling and macOS regression tests.

File	Description
test-parity/​known_failures.json	Allow-lists the macOS fixture on Linux.
test-files/​test_issue_11661_textfield_baseline.ts	Adds a visual smoke fixture.
crates/​perry-ui-macos/​tests/​native_textfield_newlines.rs	Tests single-line input behavior.
crates/​perry-ui-macos/​tests/​native_textfield_cell.rs	Tests baseline, padding, sizing, and backgrounds.
crates/​perry-ui-macos/​tests/​native_text_color.rs	Updates label-color regression context.
crates/​perry-ui-macos/​tests/​native_dynamic_colors.rs	Uses typed CGColor pointers.
crates/​perry-ui-macos/​src/​widgets/​textfield.rs	Adds factory-built inset cells and newline filtering.
crates/​perry-ui-macos/​src/​widgets/​text.rs	Builds labels through PerryLabel.
crates/​perry-ui-macos/​src/​widgets/​securefield.rs	Applies equivalent secure-field behavior.
crates/​perry-ui-macos/​src/​widgets/​padding.rs	Insets drawing and editing frames.
crates/​perry-ui-macos/​src/​widgets/​attributed_text.rs	Reuses the factory-built label path.
crates/​perry-ui-macos/​src/​srgb.rs	Defines a correctly encoded CGColor pointer.
crates/​perry-ui-macos/​Cargo.toml	Registers the new native tests.
changelog.d/​11665-macos-textfield-baseline.md	Documents the fixes.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[steinybot]

@coderabbitai

The pull request includes changes not required by #11661. TextField and SecureField now strip or replace line breaks and suppress line-break commands. The new native_textfield_newlines test verifies that behavior. The pull request also changes layer-background handling, the CGColor FFI type, and parity tracking for the new smoke test.

These are in scope. Dropping usesSingleLineMode is the fix for #11661. Without it a stock NSTextField keeps pasted and Option-Return newlines and grows taller, so the field keeps itself to one line. The layer background is part of the padding fix, because the cell background stopped at the padding. The CGColor type is needed because that layer path panicked in a debug build, and native_dynamic_colors failed on it. The known_failures.json entry is the Linux parity skip for the new smoke test, the same as test_issue_10155_textfield_singleline.

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.

The functions without docstrings are AppKit overrides (cellClass, setStringValue:, drawInteriorWithFrame:inView: and so on) and test code. Apple documents the overrides, so a docstring would repeat the selector.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
ast-grep outline crates/perry-ui-macos/src/widgets/textfield.rs --items all
ast-grep outline crates/perry-ui-macos/src/widgets/padding.rs --items all
ast-grep outline crates/perry-ui-macos/src/widgets/securefield.rs --items all
ast-grep outline crates/perry-ui-macos/src/srgb.rs --items all
sed -n '1,270p' crates/perry-ui-macos/src/widgets/textfield.rs
sed -n '560,615p' crates/perry-ui-macos/src/widgets/textfield.rs
sed -n '1,230p' crates/perry-ui-macos/src/widgets/padding.rs
sed -n '1,135p' crates/perry-ui-macos/src/widgets/securefield.rs
sed -n '1,95p' crates/perry-ui-macos/src/srgb.rs
rg -n -C6 'test_issue_11661_textfield_baseline|test_issue_10155_textfield_singleline' test-parity/known_failures.json
ast-grep outline crates/perry-ui-macos/tests/native_textfield_newlines.rs --items all
sed -n '1,240p' crates/perry-ui-macos/tests/native_textfield_newlines.rs

Length of output: 47903

---

@steinybot, thanks for the clarification. I withdraw both concerns.

• Scope: The newline handling preserves one-line input behavior after removing usesSingleLineMode. The layer background fills the padding area. The typed CGColor pointer provides the encoding that the layer API expects. The known_failures.json entry is Linux-only and matches the existing test_issue_10155_textfield_singleline entry. These changes support #11661.
• Docstrings: The coverage percentage alone does not establish a documentation defect here. The AppKit overrides do not need docstrings that only repeat the selector contract. The code already explains the non-obvious padding and encoding decisions. I do not request redundant docstrings for these overrides or test functions.

---

✏️ Learnings added

Learnt from: steinybot
URL: https://github.com/PerryTS/perry/pull/11665

Timestamp: 2026-10-01T00:43:01.100Z
Learning: In Perry's Rust macOS UI implementation, removing usesSingleLineMode is part of the TextField baseline fix for #11661. TextField and SecureField must then enforce one-line input behavior explicitly because stock AppKit fields can retain pasted and Option-Return newlines and grow taller. The newline-handling tests are supporting regression coverage, not unrelated scope.

---

Learnt from: steinybot
URL: https://github.com/PerryTS/perry/pull/11665

Timestamp: 2026-10-01T00:43:01.100Z
Learning: In crates/perry-ui-macos/src/widgets/textfield.rs, layer-backed backgrounds are part of the padding fix because cell-drawn backgrounds fill only the text area inside the padding. In crates/perry-ui-macos/src/srgb.rs, the opaque CGColor pointer must encode as ^{CGColor=} for CALayer color properties; a c_void pointer encodes as ^v and causes debug message validation to reject the send.

---

Learnt from: steinybot
URL: https://github.com/PerryTS/perry/pull/11665

Timestamp: 2026-10-01T00:43:01.100Z
Learning: For Perry's Rust AppKit overrides, such as cellClass, setStringValue:, and drawInteriorWithFrame:inView:, do not request docstrings that merely repeat Apple's selector documentation. Do not treat missing docstrings on overrides and test functions as a defect solely to satisfy a coverage percentage. Assess whether non-obvious Perry-specific behavior needs explanation.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

[steinybot]

@coderabbitai run pre-merge checks

[coderabbitai]

✅ Actions performed

Running Pre-merge checks. Results will be posted in the summary comment once complete.