diff --git a/changelog.d/10862-textsetcolor-label.md b/changelog.d/10862-textsetcolor-label.md new file mode 100644 index 0000000000..5ef45a6d1b --- /dev/null +++ b/changelog.d/10862-textsetcolor-label.md @@ -0,0 +1 @@ +Fixed macOS `textSetColor` having no visible effect on a `Text()` label. `perry-ui-macos`'s `install_label_cell` restored the factory label's text with `setAttributedStringValue:`, baking the default `labelColor` into the field. An `NSTextField` that holds an attributed string ignores `setTextColor:` — the string's baked color attribute wins — so `perry_ui_text_set_color` was silently overridden and every label rendered in `labelColor`. The text is now restored as a plain `stringValue` with `labelColor` set explicitly, so `setTextColor:` is honored while the default appearance is unchanged. Covered by a `perry-ui-macos` regression that reads back the label's foreground color after `setTextColor:`. diff --git a/crates/perry-ui-macos/Cargo.toml b/crates/perry-ui-macos/Cargo.toml index 31fce17283..d7528f407d 100644 --- a/crates/perry-ui-macos/Cargo.toml +++ b/crates/perry-ui-macos/Cargo.toml @@ -90,3 +90,8 @@ harness = false name = "native_widget_max_width" path = "tests/native_widget_max_width.rs" harness = false + +[[test]] +name = "native_text_color" +path = "tests/native_text_color.rs" +harness = false diff --git a/crates/perry-ui-macos/src/widgets/padding.rs b/crates/perry-ui-macos/src/widgets/padding.rs index e4e88ae435..f53179f255 100644 --- a/crates/perry-ui-macos/src/widgets/padding.rs +++ b/crates/perry-ui-macos/src/widgets/padding.rs @@ -1,7 +1,9 @@ use objc2::rc::Retained; use objc2::runtime::{AnyClass, AnyObject}; use objc2::{define_class, msg_send, DefinedClass, MainThreadOnly}; -use objc2_app_kit::{NSEvent, NSSecureTextFieldCell, NSText, NSTextField, NSTextFieldCell, NSView}; +use objc2_app_kit::{ + NSColor, NSEvent, NSSecureTextFieldCell, NSText, NSTextField, NSTextFieldCell, NSView, +}; use objc2_core_foundation::CGRect; use objc2_foundation::{MainThreadMarker, NSEdgeInsets, NSObjectProtocol}; use std::cell::Cell; @@ -206,7 +208,13 @@ pub(crate) fn install_secure_text_field_cell(field: &NSTextField, mtm: MainThrea /// Install at label creation, before callers apply attributed text or styles. /// Keep the factory label's text, font and line-breaking defaults. pub(crate) fn install_label_cell(field: &NSTextField, mtm: MainThreadMarker) { - let value = field.attributedStringValue(); + // Restore the text as a plain stringValue, never the factory label's + // attributedStringValue. An NSTextField holding an attributed string + // ignores setTextColor: — the string's baked-in color attribute wins — so + // an attributedStringValue here silently defeats textSetColor (#10856). + // The font is restored separately below, and labelColor is set explicitly + // to keep the label's default appearance. + let text = field.stringValue(); let font = field.font(); let original = field.cell().expect("label has a cell"); install_text_field_cell(field, mtm); @@ -216,7 +224,8 @@ pub(crate) fn install_label_cell(field: &NSTextField, mtm: MainThreadMarker) { field.setSelectable(false); field.setDrawsBackground(false); field.setFont(font.as_deref()); - field.setAttributedStringValue(&value); + field.setStringValue(&text); + field.setTextColor(Some(&NSColor::labelColor())); if let Some(cell) = field.cell() { cell.setWraps(original.wraps()); cell.setScrollable(original.isScrollable()); diff --git a/crates/perry-ui-macos/tests/native_text_color.rs b/crates/perry-ui-macos/tests/native_text_color.rs new file mode 100644 index 0000000000..f0a539d16e --- /dev/null +++ b/crates/perry-ui-macos/tests/native_text_color.rs @@ -0,0 +1,64 @@ +// #10856 — textSetColor must color a Text() label. install_label_cell used to +// restore the text with setAttributedStringValue:, baking the default +// labelColor into the field; an NSTextField holding an attributed string +// ignores setTextColor:, so the color was silently overridden. This drives the +// real create + set_color path (the same entry points textSetColor lowers to) +// and asserts the label's rendered foreground color is the requested red. +// +// AppKit requires the main thread, so this is a harness = false binary whose +// main() is the process main thread — the pattern the other native_widget_* +// tests use. The default #[test] harness runs on a worker thread and cannot. + +#[cfg(target_os = "macos")] +fn main() { + use objc2::msg_send; + use objc2::rc::Retained; + use objc2_app_kit::{NSColorSpace, NSTextField}; + use objc2_foundation::{MainThreadMarker, NSString}; + use perry_ui_macos::widgets; + + if std::env::args().any(|arg| arg == "--list") { + println!("native_text_color: test"); + return; + } + let _mtm = MainThreadMarker::new().expect("native text-color test runs on the main thread"); + + let text = "hi"; + let string = perry_runtime::string::js_string_from_bytes(text.as_ptr(), text.len() as u32); + let handle = widgets::text::create(string.cast()); + widgets::text::set_color(handle, 1.0, 0.0, 0.0, 1.0); + + let view = widgets::get_widget(handle).expect("Text widget is registered"); + let field = unsafe { &*(Retained::as_ptr(&view) as *const NSTextField) }; + + // A plain-string field renders in its textColor; attributedStringValue + // synthesizes that color at index 0. The old bug left a baked labelColor + // here instead of the red just set. + let attributed = field.attributedStringValue(); + let key = NSString::from_str("NSColor"); + let color: Option> = unsafe { + msg_send![ + &*attributed, + attribute: &*key, + atIndex: 0usize, + effectiveRange: std::ptr::null_mut::() + ] + }; + let color = color.expect("label carries a foreground color at index 0"); + let srgb = color + .colorUsingColorSpace(&NSColorSpace::sRGBColorSpace()) + .expect("foreground color converts to sRGB"); + let (r, g, b) = ( + srgb.redComponent(), + srgb.greenComponent(), + srgb.blueComponent(), + ); + assert!( + r > 0.9 && g < 0.1 && b < 0.1, + "label rendered ({r:.3}, {g:.3}, {b:.3}), not red — setTextColor was overridden (#10856)" + ); + println!("PASS native text-color: textSetColor colors a Text label"); +} + +#[cfg(not(target_os = "macos"))] +fn main() {}