Fix Quick Look extension bugs (blank preview, entitlements, baseURL)#368
Merged
Conversation
New tests that expose five bugs in the Quick Look extension: MPQuickLookRendererTests.m: - testRenderMarkdownFromURLWithLatin1Encoding: FAILS because renderMarkdownFromURL: only tries UTF-8, no encoding fallback - testRenderMarkdownFromURLWithUTF16Encoding: same issue - testRenderMarkdownFromURLUTF8TakesPrecedenceOverFallback: PASSES (regression guard) MPPreviewViewControllerTests.m (new file): - testPreviewViewControllerConformsToWKNavigationDelegate: FAILS - testPreviewViewControllerImplementsDidFinishNavigation: FAILS - testPreviewViewControllerImplementsDidFailNavigation: FAILS - testPreviewViewControllerImplementsDidFailProvisionalNavigation: FAILS - testCompletionHandlerIsNotCalledSynchronously: FAILS (handler called immediately) - testCompletionHandlerIsEventuallyCalledAfterNavigation: FAILS (no deferred call) - testLoadViewCreatesWKWebViewWithNavigationDelegate: FAILS (delegate not set) - testPreferredContentSizeIsSet: FAILS (NSZeroRect used) project.pbxproj: - Enable ENABLE_QUICKLOOK_TESTS=1 in MacDownTests build settings - Add HEADER_SEARCH_PATHS for MacDownCore and MacDownQuickLook - Wire Quick Look source and test files into Compile Sources phase - Add MacDownCore and MacDownQuickLook groups to project navigator Related to #367
PreviewViewController.h/m: - Conform to WKNavigationDelegate so the completion handler can be deferred until WKWebView finishes rendering - Store QL completion handler in pendingHandler property and fire it from didFinishNavigation: instead of calling it synchronously (fixes blank Quick Look preview pane) - Implement didFailNavigation:withError: and didFailProvisionalNavigation:withError: to propagate navigation errors (e.g. sandbox violations) back to Quick Look - Set webView.navigationDelegate = self in loadView - Use baseURL:nil — the extension sandbox only grants access to the specific file URL, not its parent directory - Set preferredContentSize to 800x600 so Quick Look has a non-zero initial size MacDownQuickLook.entitlements: - Add com.apple.security.files.user-selected.read-only (needed to read the file being previewed) - Add com.apple.security.network.client (required by WKWebView WebContent process IPC inside an extension sandbox) MPQuickLookRenderer.m: - Add encoding fallback in renderMarkdownFromURL:error: — if UTF-8 decoding fails, retry with usedEncoding: auto-detection for files encoded as Latin-1, UTF-16, etc. Related to #367
The hoedown_escape_html function (used in the blockcode renderer callback) is declared in escape.h but was not explicitly imported. This compiled in the main app target due to transitive includes but triggers -Wimplicit-function-declaration when the file is compiled in the test target (macOS 26 SDK). Related to #367
- quick-look-xcode-setup.md: document that ENABLE_QUICKLOOK_TESTS is now configured and three test files are wired into the project - test_coverage_improvement_plan.md: add Quick Look test files to the coverage inventory Related to #367
PreviewViewController.m uses WKWebView and WKWebViewConfiguration, which require WebKit.framework. The main app target already links it but the test target did not, causing linker errors when compiling PreviewViewController.m in the test target. Related to #367
Contributor
Code Coverage ReportCurrent Coverage: 0.00% Coverage Details (Summary) |
If the view controller is deallocated while pendingHandler is still set (e.g. Quick Look tears down the extension before WKWebView finishes loading), drain the handler with a cancellation error so Quick Look doesn't hang waiting for a callback that will never come. Related to #367
MPQuickLookRenderer.m: - Add ISO Latin-1 as a last-resort encoding fallback. The usedEncoding: auto-detection requires markers like BOM to work; single-byte encodings (Latin-1, Windows-1252) lack markers and fail auto-detection. Latin-1 can decode any byte sequence so it serves as a universal fallback. MPQuickLookRendererTests.m: - Fix testPrismScriptsIncluded to check for language-javascript class rather than Prism script content. The test bundle does not include Prism resource files, so the renderer correctly omits the scripts. The language class on the code block is always present regardless of bundle resources. Related to #367
This was referenced Jun 29, 2026
schuyler
added a commit
that referenced
this pull request
Jun 30, 2026
## Summary - Add MacDownCore (framework) and MacDownQuickLook (app extension) as native targets in the Xcode project - Wire CocoaPods dependencies for both targets - Embed the Quick Look extension in the main app bundle - Remove `public.text` from `QLSupportedContentTypes` so the extension only claims markdown files ## Background The Quick Look source code was merged in PRs #299, #368, and #385 but the Xcode project was never configured to build it. This caused 3000.0.6 to register the QL handler without providing a working preview — worse than the macOS default. ## Key decisions - **Resource copying via script phase**: MacDownCore copies Styles and Prism resources via its own shell script build phase rather than reusing MacDown's PBXFileReferences. Those refs are declared outputs of MacDown's "Fetch Prism Resources" and "Transpile Styles" script phases, which would create a dependency cycle. - **No MacDown→MacDownCore dependency**: The main app only depends on MacDownQuickLook. MacDownCore is embedded inside the extension only, since only the extension uses it. - **Per-configuration bundle IDs**: Debug uses `-debug` suffix to match the parent app's pattern. ## Test plan - [ ] `xcodebuild build` succeeds (verified locally) - [ ] 1003 tests pass with 0 unexpected failures (verified locally) - [ ] `.app` bundle contains `MacDownQuickLook.appex` in `Contents/PlugIns/` (verified locally) - [ ] `MacDownQuickLook.appex` contains `MacDownCore.framework` with `Styles/` and `Prism/` resources (verified locally) - [ ] Quick Look preview renders markdown files in Finder - [ ] Quick Look does NOT activate for `.txt`, `.csv`, or other non-markdown text files Related to #284
schuyler
added a commit
that referenced
this pull request
Jun 30, 2026
## Summary - Add MacDownCore (framework) and MacDownQuickLook (app extension) as native targets in the Xcode project - Wire CocoaPods dependencies for both targets - Embed the Quick Look extension in the main app bundle - Remove `public.text` from `QLSupportedContentTypes` so the extension only claims markdown files ## Background The Quick Look source code was merged in PRs #299, #368, and #385 but the Xcode project was never configured to build it. This caused 3000.0.6 to register the QL handler without providing a working preview — worse than the macOS default. ## Key decisions - **Resource copying via script phase**: MacDownCore copies Styles and Prism resources via its own shell script build phase rather than reusing MacDown's PBXFileReferences. Those refs are declared outputs of MacDown's "Fetch Prism Resources" and "Transpile Styles" script phases, which would create a dependency cycle. - **No MacDown→MacDownCore dependency**: The main app only depends on MacDownQuickLook. MacDownCore is embedded inside the extension only, since only the extension uses it. - **Per-configuration bundle IDs**: Debug uses `-debug` suffix to match the parent app's pattern. ## Test plan - [ ] `xcodebuild build` succeeds (verified locally) - [ ] 1003 tests pass with 0 unexpected failures (verified locally) - [ ] `.app` bundle contains `MacDownQuickLook.appex` in `Contents/PlugIns/` (verified locally) - [ ] `MacDownQuickLook.appex` contains `MacDownCore.framework` with `Styles/` and `Prism/` resources (verified locally) - [ ] Quick Look preview renders markdown files in Finder - [ ] Quick Look does NOT activate for `.txt`, `.csv`, or other non-markdown text files Related to #284
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
files.user-selected.read-onlyandnetwork.clientbaseURL:nilfor sandbox safety (extension only has access to the specific file URL)WKNavigationDelegateerror handling (didFailNavigation:,didFailProvisionalNavigation:)MPQuickLookRenderer(UTF-8 → auto-detection)ENABLE_QUICKLOOK_TESTSin Xcode project and wire Quick Look source/test files into buildRelated Issue
Related to #367
Manual Testing Plan
Quick Look extensions run in a sandboxed, out-of-process environment that unit tests cannot replicate. Manual testing is recommended:
.mdfile in Finder, press Space. Expected: rendered HTML appears, not a blank pane.denymessages filtered toMacDownQuickLook.iconv -f UTF-8 -t ISO-8859-1 test.md > latin1.md). Preview it. Expected: renders correctly.Review Notes
hoedown/escape.himport exposed by compilingMPQuickLookRenderer.min test target