Repository navigation
Fix .ts files opening as media player in file preview - #4380
philip-zhan wants to merge 3 commits into
Conversation
Reproduces the bug where TypeScript files open in the AVPlayer-backed
media preview because macOS's UTType("ts") resolves to MPEG-2 transport
stream, and the resolver checks media-mode before the text-extension
allowlist.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The resolver was checking media UTType conformance before consulting the
text-extension allowlist. On macOS, UTType("ts") resolves to MPEG-2
transport stream (a movie type), so TypeScript files were routed to the
AVPlayer-backed media preview even though "ts" is explicitly listed in
textExtensions.
Reorder both initialResolution and resolvedResolution so the explicit
text-file allowlist wins over UTType-based media classification.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@philip-zhan 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 (2)
📝 WalkthroughWalkthroughFilePreviewKindResolver reorders text-file detection to take precedence over media-type inference in two resolver methods. The ChangesText-first file preview resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 16 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (16 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
Confidence Score: 5/5Safe to merge — the change is a minimal, targeted reordering of two guard blocks with no new state or side effects. Both resolution functions now correctly route TypeScript files to the text editor. The ext == plist early-exit in initialResolution is still ordered before knownTextFile, so binary-plist sniffing is unaffected. The looksLikeBinaryPropertyList guard in resolvedResolution is likewise unchanged. A regression test exercises the fixed path end-to-end with a real temporary file, and the existing testBinaryPlistDoesNotOpenAsEditableText test continues to guard the plist path. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[URL with extension] --> B{initialResolution}
A --> C{resolvedResolution}
B --> B1{ext == plist?}
B1 -->|Yes| B2[.needsSniff]
B1 -->|No| B3{knownTextFile extension in textExtensions?}
B3 -->|Yes| B4[.resolved .text]
B3 -->|No| B5{UTType conforms to media?}
B5 -->|Yes| B6[.resolved .video/.audio]
B5 -->|No| B7[.needsSniff]
C --> C1{ext==plist AND binary content?}
C1 -->|Yes| C2[.resolved .quickLook]
C1 -->|No| C3{knownTextFile with content type?}
C3 -->|Yes| C4[.resolved .text]
C3 -->|No| C5{contentTypes loop media conformance?}
C5 -->|Match| C6[.resolved .video/.audio]
C5 -->|No match| C7[.needsSniff]
Reviews (2): Last reviewed commit: "Preserve binary plist QuickLook initial ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -718,10 +722,6 @@ enum FilePreviewKindResolver { | |||
| return .needsSniff | |||
| } | |||
|
|
|||
| if knownTextFile(url: url, includeResourceContentType: false) { | |||
| return .resolved(.text) | |||
| } | |||
|
|
|||
| return .needsSniff | |||
| } | |||
There was a problem hiding this comment.
.plist initial-mode regression from reorder
Moving knownTextFile to the top of initialResolution causes every .plist file — including binary ones — to immediately resolve to .text, because "plist" is present in textExtensions. The original code had the if ext == "plist" { return .needsSniff } guard specifically to force content sniffing before committing to text mode. That guard is now dead code: knownTextFile short-circuits to .resolved(.text) before the guard is ever reached.
The pre-existing test testBinaryPlistDoesNotOpenAsEditableText asserts XCTAssertEqual(FilePreviewKindResolver.initialMode(for: url), .quickLook), which will now fail. The resolvedResolution path is unaffected (looksLikeBinaryPropertyList still fires first there), but the initial speculative classification shown to the user regresses.
| private static func initialResolution(for url: URL) -> Resolution { | |
| let ext = url.pathExtension.lowercased() | |
| if ext == "plist" { | |
| return .needsSniff | |
| } | |
| if knownTextFile(url: url, includeResourceContentType: false) { | |
| return .resolved(.text) | |
| } | |
| if let type = UTType(filenameExtension: ext), | |
| let mediaMode = mediaMode(for: type) { | |
| return .resolved(mediaMode) | |
| } | |
| return .needsSniff | |
| } |
Greptile flagged that moving knownTextFile before the plist guard in initialResolution dead-coded the `ext == "plist"` branch, since "plist" is in textExtensions. That would cause binary plists to flash as .text in initialMode (regressing testBinaryPlistDoesNotOpenAsEditableText) before resolvedResolution sniffed them back to .quickLook. Check the plist extension first so plists always go through .needsSniff and let resolvedResolution decide based on the bplist00 header. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Thanks for this! .ts files now open as text previews landed on main in #4924. You opened this first, so you got there first. Closing since main covers it now. |
2 participants
Summary
.ts(TypeScript) files were opening in the AVPlayer-backed media preview instead of the text editorUTType(filenameExtension: "ts")resolves topublic.mpeg-2-transport-stream, which conforms to.movie.FilePreviewKindResolverwas checking media UTType conformance before consulting its own text-extension allowlist (which does contain"ts"), so the media branch always woninitialResolutionandresolvedResolutioninSources/Panels/FilePreviewPanel.swiftso the explicit text-file allowlist wins over UTType-based media classificationTwo-commit structure per the regression test policy: commit 1 adds the failing test, commit 2 applies the fix.
Test plan
testTypeScriptExtensionResolvesToTextNotMediaPlayerred on commit 1, green on commit 2.tsfile in the cmux file preview and confirm it opens in the text editor with thedoc.texticon.mov/.mp4and confirm media playback still works.plistand confirm it still routes to QuickLook (not text)🤖 Generated with Claude Code
Summary by cubic
Fixes
.tsfiles opening in the media player by prioritizing the text extension allowlist over UTType-based media detection. TypeScript files now open in the text editor; preserves QuickLook for binaryplistfiles and media behavior is unchanged.FilePreviewPanel.swift:plistcheck first, thenknownTextFile(...), then media UTType in the appropriate paths.testTypeScriptExtensionResolvesToTextNotMedia.Written for commit 7f75b05. Summary will update on new commits. Review in cubic
Summary by CodeRabbit