diff --git a/CHANGELOG.md b/CHANGELOG.md index 79cf01a9..e5cd6d28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ### Fixed - Fix the Insert Table toolbar button doing nothing when the editor pane had focus, and corrupting the document when clicked repeatedly (a second table was inserted inside the first table's cell); every click now inserts a clean, well-separated table regardless of which pane has focus (#278) -- thanks @rcuisnier for the report! +- Fix auto-reload silently breaking after an external editor's atomic save by making the local-volume watcher check fall back to the parent directory when the file is transiently missing; also guard resource-file watchers against remote volumes and tear down any prior watcher before re-arming (#478) - Fix the Preview pane still auto-scrolling toward the editor when typing after Sync Panes was turned off mid-session; toggling Sync Panes now takes effect immediately (settling the panes on disable, re-syncing on enable) without needing to reopen the document (#441) -- thanks @gregwillits! - Fix blank preview when opening saved or externally-originated documents: the real document file is no longer used as the preview's base resource, which WebKit on macOS 26 can silently refuse to load (e.g. files with the execute bit set by sync clients like OneDrive, or stale TCC/provenance state) (#431, #405) -- thanks @maskedspitz, @songjianbupt, and @craigrodger for the diagnosis! - Improve render-path responsiveness: single-pass word/character counting, cached body-extraction regex, and bounded renderer polling with cancellation (#388) diff --git a/MacDown/Code/Document/MPDocument.m b/MacDown/Code/Document/MPDocument.m index 930e1565..92511181 100644 --- a/MacDown/Code/Document/MPDocument.m +++ b/MacDown/Code/Document/MPDocument.m @@ -3856,11 +3856,14 @@ + (NSString *)toggleCheckboxAtIndex:(NSUInteger)index inMarkdown:(NSString *)mar - (void)startFileWatching { + // Tear down any previously-armed watcher first, so an early return below + // (nil URL, or a path that cannot be watched) can never leak a stale + // watcher for an old session. Related to #478. + [self stopFileWatching]; + if (!self.fileURL || !self.fileURL.isFileURL) return; - [self stopFileWatching]; - if (![MPFileWatcher canWatchPath:self.fileURL.path]) return; diff --git a/MacDown/Code/Utility/MPFileWatcher.h b/MacDown/Code/Utility/MPFileWatcher.h index f51022e2..304ea4be 100644 --- a/MacDown/Code/Utility/MPFileWatcher.h +++ b/MacDown/Code/Utility/MPFileWatcher.h @@ -16,7 +16,11 @@ /// YES if currently watching. @property (nonatomic, readonly, getter=isWatching) BOOL watching; -/// YES if the path is on a local volume that should receive vnode watchers. +/// YES if the path can receive vnode watchers: it is non-nil, non-empty, and +/// resides on a local volume. When the file itself does not exist (e.g. during +/// an external editor's atomic rename-replace save) the locality check falls +/// back to the parent directory. Returns NO for nil, empty, non-local, or +/// otherwise unresolvable paths. + (BOOL)canWatchPath:(NSString *)path; /// Create a watcher for the given path. Calls handler on the main queue diff --git a/MacDown/Code/Utility/MPFileWatcher.m b/MacDown/Code/Utility/MPFileWatcher.m index dd3b1170..026304df 100644 --- a/MacDown/Code/Utility/MPFileWatcher.m +++ b/MacDown/Code/Utility/MPFileWatcher.m @@ -21,11 +21,26 @@ + (BOOL)canWatchPath:(NSString *)path if (!path.length) return NO; - NSURL *url = [NSURL fileURLWithPath:path]; + // NSURLVolumeIsLocalKey can only be read from a URL that resolves to an + // existing item. During an external editor's atomic rename-replace save the + // file transiently disappears, so probing the file itself would wrongly + // report the path as non-local and disable auto-reload. Fall back to the + // parent directory's volume in that case. Related to #478. + NSFileManager *fileManager = [NSFileManager defaultManager]; + NSString *probePath = path; + if (![fileManager fileExistsAtPath:probePath]) + probePath = [path stringByDeletingLastPathComponent]; + if (!probePath.length || ![fileManager fileExistsAtPath:probePath]) + return NO; + + NSURL *url = [NSURL fileURLWithPath:probePath]; NSNumber *isLocal = nil; + NSError *error = nil; if (![url getResourceValue:&isLocal - forKey:NSURLVolumeIsLocalKey error:NULL]) + forKey:NSURLVolumeIsLocalKey error:&error]) { + NSLog(@"MPFileWatcher: cannot determine volume locality for %@: %@", + probePath, error.localizedDescription); return NO; } diff --git a/MacDown/Code/Utility/MPResourceWatcherSet.m b/MacDown/Code/Utility/MPResourceWatcherSet.m index de98e55c..f38b99d2 100644 --- a/MacDown/Code/Utility/MPResourceWatcherSet.m +++ b/MacDown/Code/Utility/MPResourceWatcherSet.m @@ -57,6 +57,12 @@ - (void)updateWatchedPaths:(NSSet *)paths - (void)addWatcherForPath:(NSString *)path { + // Skip paths that cannot receive vnode watchers (nil/empty or on a remote + // volume) before constructing a watcher, mirroring the guard in + // -[MPDocument startFileWatching]. Related to #478. + if (![MPFileWatcher canWatchPath:path]) + return; + __weak MPResourceWatcherSet *weakSelf = self; NSString *watchedPath = [path copy]; diff --git a/MacDownTests/MPFileWatcherTests.m b/MacDownTests/MPFileWatcherTests.m index bd0e916d..ad922f7e 100644 --- a/MacDownTests/MPFileWatcherTests.m +++ b/MacDownTests/MPFileWatcherTests.m @@ -87,6 +87,32 @@ - (void)testCannotWatchNilPath XCTAssertFalse([MPFileWatcher canWatchPath:nil]); } +- (void)testCannotWatchEmptyPath +{ + XCTAssertFalse([MPFileWatcher canWatchPath:@""]); +} + +- (void)testCanWatchNonexistentLocalPath +{ + // A file that does not yet exist (e.g. mid atomic rename-replace save) on a + // local volume must still be considered watchable: the locality check falls + // back to the parent directory. Otherwise auto-reload silently dies during + // an external editor's save. Related to #478. + NSString *path = [self.testDirectory stringByAppendingPathComponent:@"gone.txt"]; + XCTAssertFalse([self.fileManager fileExistsAtPath:path]); + XCTAssertTrue([MPFileWatcher canWatchPath:path]); +} + +- (void)testCannotWatchPathWithNonexistentParent +{ + // Neither the file nor its parent directory exists -> nothing to probe for + // volume locality, so watching is not possible. + NSString *path = [[self.testDirectory + stringByAppendingPathComponent:@"missing-dir"] + stringByAppendingPathComponent:@"file.txt"]; + XCTAssertFalse([MPFileWatcher canWatchPath:path]); +} + #pragma mark - Event Handling - (void)testHandlerCalledOnFileWrite diff --git a/MacDownTests/MPResourceWatcherSetTests.m b/MacDownTests/MPResourceWatcherSetTests.m index 439fd317..b71ed618 100644 --- a/MacDownTests/MPResourceWatcherSetTests.m +++ b/MacDownTests/MPResourceWatcherSetTests.m @@ -122,6 +122,15 @@ - (void)testSkipsNonexistentFiles XCTAssertEqual(self.watcherSet.watchedPaths.count, 0u); } +- (void)testSkipsUnwatchablePath +{ + // An unwatchable path (here an empty string, but in practice a remote + // volume) must be rejected before a watcher is constructed and never + // stored. Related to #478. + [self.watcherSet updateWatchedPaths:[NSSet setWithObject:@""]]; + XCTAssertEqual(self.watcherSet.watchedPaths.count, 0u); +} + - (void)testDelegateCalledOnChange { NSString *path = [self createTestFileWithName:@"watch.png"];