Repository navigation
Fix macOS 27 crash opening browser panel (zero-size SF Symbol rasterization) - #5842
matheustimbo wants to merge 2 commits into
Conversation
|
@matheustimbo is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughAdds Image.cmuxSymbolPixelSize(_:weight:) to size SF Symbols via an explicit centered frame and replaces font-based SF Symbol sizing with this helper across BrowserPanelView toolbar icons and one ContentView usage. ChangesBrowser Toolbar SF Symbol Sizing
Sequence Diagram(s)sequenceDiagram
participant ViewUsingImage
participant Image.cmuxSymbolPixelSize
participant SwiftUIRenderer
ViewUsingImage->>Image.cmuxSymbolPixelSize: request symbol with size & weight
Image.cmuxSymbolPixelSize->>SwiftUIRenderer: provide resizable symbol with explicit frame
SwiftUIRenderer->>ViewUsingImage: render non-zero-rasterized symbol
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR fixes the macOS 26+ / macOS 27 crash that fires when the browser panel toolbar is first laid out: SF Symbols sized via
Confidence Score: 5/5Safe to merge — the change is a targeted, mechanically straightforward modifier swap with no new state, no layout mutation during rendering, and no behavioral changes beyond eliminating the zero-size rasterization path. Every changed site is a pure view-modifier substitution: No files require special attention. The PR author's note that the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Image(systemName:)"] --> B["cmuxSymbolPixelSize(size, weight)"]
B --> B1[".resizable()"]
B1 --> B2[".scaledToFit()"]
B2 --> B3[".fontWeight(weight)"]
B3 --> B4[".frame(size × size)"]
B4 --> C[".symbolRenderingMode(.monochrome) ①"]
C --> D[".cmuxFlatSymbolColorRendering() ①"]
D --> E[".foregroundStyle(color)"]
E --> F[".frame(buttonSize × buttonSize) — hit target"]
subgraph macOS_26_path["macOS 26+ layout pass"]
G["font metrics not yet resolved"] --> H["⚠ .font() rasterizes at 0×0"]
H --> I["NSInvalidArgumentException → crash"]
J["explicit .frame() rasterizes at size × size"] --> K["✅ always positive — no crash"]
end
B4 -.->|fixes| K
A -.->|old path| H
note1["① env-propagated: correctly reaches Image inside fixed frame"]
Reviews (3): Last reviewed commit: "Move cmuxSymbolPixelSize to a shared fil..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| extension Image { | ||
| /// Sizes an SF Symbol from an explicit frame rather than font metrics. | ||
| /// | ||
| /// A `.font(.system(size:))`-sized symbol can rasterize at 0×0 during a | ||
| /// transient layout pass (e.g. while a panel's hosting view is first added | ||
| /// to the hierarchy, before font metrics resolve). macOS 26+ rejects a | ||
| /// zero raster size with an uncaught `NSInvalidArgumentException` | ||
| /// (`targetSizeInPoints.width>0 && targetSizeInPoints.height>0`), which | ||
| /// AppKit turns into a hard crash. Driving the raster size from a fixed | ||
| /// frame keeps it positive across every layout pass. Mirrors the fix in | ||
| /// `SidebarHelpMenuButton` (#5670). | ||
| func cmuxSymbolPixelSize(_ size: CGFloat, weight: Font.Weight = .regular) -> some View { | ||
| self | ||
| .resizable() | ||
| .scaledToFit() | ||
| .fontWeight(weight) | ||
| .frame(width: size, height: size, alignment: .center) | ||
| } | ||
| } |
There was a problem hiding this comment.
cmuxSymbolPixelSize is internal (no access modifier), making it siloed in BrowserPanelView.swift but technically visible across the whole module. SidebarHelpMenuButton in ContentView.swift (lines 14554–14566) already inlines the exact same four-step pattern (resizable / scaledToFit / fontWeight / frame) without knowing this helper exists, which is a direct consequence of the discoverability gap. The PR description explicitly says the .font(.system(size:)) pattern exists in other panels app-wide, so having the canonical helper live in a panel view rather than a shared Image+CMux.swift or ViewModifiers+SymbolSize.swift file means future adopters will keep re-inventing it inline. Moving it to a shared utilities file would also let you retroactively update SidebarHelpMenuButton to use the same abstraction.
Rule Used: Flag Swift changes that add too much unrelated res... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Good call — addressed in the follow-up commit: moved cmuxSymbolPixelSize to the shared RenderableSystemSymbol.swift and updated SidebarHelpMenuButton to use it, removing the duplicated inline pattern. No behavior change.
…zation) Follow-up to manaflow-ai#5670. The browser panel toolbar sized its SF Symbols with `.font(.system(size:weight:))`. When the panel's SwiftUI hosting view is added to the hierarchy during a layout follow-up (`Workspace.flushWorkspaceWindowLayouts`), font metrics aren't resolved yet, so the symbols rasterize at 0×0 — which macOS 26+ rejects with an uncaught `NSInvalidArgumentException` ("targetSizeInPoints.width>0 && targetSizeInPoints.height>0"), crashing the app the moment a link is cmd-clicked (which opens the browser). Captured glyph: `wrench.and.screwdriver` (the default dev-tools button), but every `.font`-sized toolbar symbol is exposed. Add an `Image.cmuxSymbolPixelSize(_:weight:)` helper that drives the raster size from an explicit frame via `.resizable().scaledToFit()` (same approach as the SidebarHelpMenuButton fix in manaflow-ai#5670), and apply it across the browser toolbar: navigation chevrons, reload/stop, screenshot camera, focus keyboard, react-grab cursor, dev-tools, profile, theme, import hint, and the secure lock badge. Visual output is unchanged. Fixes manaflow-ai#5841
b9693fe to
8d93a74
Compare
…tton Addresses review feedback on the helper's discoverability: `cmuxSymbolPixelSize` lived in `BrowserPanelView.swift` (module-visible but siloed) while `SidebarHelpMenuButton` re-implemented the same four-step `resizable/scaledToFit/fontWeight/frame` pattern inline. Move the helper to the shared `RenderableSystemSymbol.swift` and have both the browser toolbar and `SidebarHelpMenuButton` use it, removing the duplicated inline pattern. No behavior change.
|
@austinywang follow-up to #5670 — same macOS 27 zero-size SF Symbol crash class, this time in the browser panel toolbar (reproduced + fixed; details in the description). Review bots are green; the Vercel checks are just the fork deploy-authorization gate (changes are macOS-app only, no web). Could you kick off CI / take a look when you have a moment? 🙏 |
|
Thanks for this! The macOS 27 zero-size SF Symbol crash in the browser panel landed on main in #6728. You opened this first, so you got there first. Closing since main covers it now. |
2 participants
Summary
Fixes #5841 — follow-up to #5670. The same macOS 26+ zero-size SF Symbol rasterization crash still fires in the browser panel toolbar: cmd-clicking a terminal link (which opens the integrated browser) crashes cmux on macOS 27.0.
Root cause
Captured under lldb on macOS 27:
BrowserPanelView's toolbar sizes its SF Symbols with.font(.system(size:weight:)). When the panel's SwiftUI hosting view is added to the hierarchy during a layout follow-up, font metrics aren't resolved yet, so the symbols rasterize at 0×0 — which macOS 26+ rejects with an uncaughtNSInvalidArgumentException, crashing the app. The captured glyph iswrench.and.screwdriver(the default dev-tools button), but all ~12.font-sized toolbar symbols are exposed.Fix
Adds an
Image.cmuxSymbolPixelSize(_:weight:)helper that drives the symbol's raster size from an explicit frame via.resizable().scaledToFit()— the same approach merged forSidebarHelpMenuButtonin #5670 — and applies it across the browser toolbar (navigation chevrons, reload/stop, screenshot camera, focus keyboard, react-grab cursor, dev-tools, profile, theme, import hint, secure lock badge). Visual output is unchanged.Verification
Built with Xcode 27 (macOS 27 SDK) and run on macOS 27.0 (26A5353q): before the fix, cmd-clicking a link crashed within ~1s, every time; after the fix, the browser panel opens and the app stays up (verified by reproducing the exact cmd-click flow under an isolated tagged build).
Note
The
.font(.system(size:))-on-symbol pattern exists in other panels app-wide; this PR fixes the browser toolbar (the reported crash). A broader sweep (or the reusable helper applied more widely) would harden the remaining surfaces — happy to follow up.🤖 Generated with Claude Code
Summary by CodeRabbit