From ad729bf454fc0c0afc6dd5a17df88bc1895d8ccc Mon Sep 17 00:00:00 2001 From: Dan Roundhill Date: Wed, 8 Jan 2025 14:57:12 -0700 Subject: [PATCH 1/2] Experimental disabling of ensureLayout calls, and process links in a bg queue instead of main thread. --- Simplenote/NSTextView+Simplenote.swift | 89 ++++++++++++------- .../NoteEditorViewController+Swift.swift | 2 - Simplenote/SPTextView.swift | 1 - 3 files changed, 58 insertions(+), 34 deletions(-) diff --git a/Simplenote/NSTextView+Simplenote.swift b/Simplenote/NSTextView+Simplenote.swift index 7d7cf9eef..b40290231 100644 --- a/Simplenote/NSTextView+Simplenote.swift +++ b/Simplenote/NSTextView+Simplenote.swift @@ -145,39 +145,66 @@ extension NSTextView { /// @objc func processLinksInDocumentAsynchronously() { - DispatchQueue.main.async(execute: processLinksInDocument) - } - - /// Processess Links in the document - /// - /// - Important: This API temporarily disables the `delegate`. - /// - Note: Invoking `checkTextInDocument` results in a call to`delegate.textDidChange`. - /// This causes the Editor to update the Note's Modification Date, and may affect the List Sort Order (!) - /// - func processLinksInDocument() { - /// Disable the Delegate: - let theDelegate = delegate - delegate = nil - - /// Issue #472: Linkification should not be undoable - undoManager?.disableUndoRegistration() - - if let textStorage = textStorage as? Storage { - - // checkTextInDocument calculate bounds for links and we need to ensure that layout is current before calling beginEditing - // otherwise it will crash trying to update layout inside beginEditing / endEditing block - ensureLayout() - textStorage.beginEditing() - checkTextInDocument(nil) - textStorage.endEditingWithoutRestyling() - } else { - checkTextInDocument(nil) + // Capture the current text and attributes to process in background + guard let textStorage = textStorage, + let attributedString = try? NSAttributedString(attributedString: textStorage.copy() as! NSAttributedString) else { + return } - undoManager?.enableUndoRegistration() - - /// Restore the Delegate - delegate = theDelegate + DispatchQueue.global(qos: .userInitiated).async { [weak self] in + // Create a data detector for links + let types = NSTextCheckingResult.CheckingType.link.rawValue + guard let detector = try? NSDataDetector(types: types) else { + return + } + + // Find all links in the text + let fullRange = NSRange(location: 0, length: attributedString.length) + let matches = detector.matches(in: attributedString.string, options: [], range: fullRange) + + // Apply the changes on the main queue + DispatchQueue.main.async { + guard let self = self, + let textStorage = self.textStorage else { + return + } + + // Disable the Delegate temporarily + let theDelegate = self.delegate + self.delegate = nil + + /// Issue #472: Linkification should not be undoable + self.undoManager?.disableUndoRegistration() + + if let storage = textStorage as? Storage { + storage.beginEditing() + + // Remove existing link attributes + textStorage.removeAttribute(.link, range: fullRange) + + // Add link attributes while preserving existing ones + for match in matches where match.resultType == .link { + if let url = match.url { + textStorage.addAttribute(.link, value: url, range: match.range) + } else { + // If no URL, create one from the matched text + let text = attributedString.string as NSString + let linkText = text.substring(with: match.range) + if let url = URL(string: linkText) { + textStorage.addAttribute(.link, value: url, range: match.range) + } + } + } + + storage.endEditingWithoutRestyling() + } + + self.undoManager?.enableUndoRegistration() + + /// Restore the Delegate + self.delegate = theDelegate + } + } } } diff --git a/Simplenote/NoteEditorViewController+Swift.swift b/Simplenote/NoteEditorViewController+Swift.swift index bdc148cbc..d61ea3810 100644 --- a/Simplenote/NoteEditorViewController+Swift.swift +++ b/Simplenote/NoteEditorViewController+Swift.swift @@ -962,8 +962,6 @@ extension NoteEditorViewController { scrollView.scrollToTop(animated: false) return } - // ensure layout to make sure that content size is updated - noteEditor.ensureLayout() scrollView.documentView?.scroll(NSPoint(x: 0, y: scrollPosition)) } diff --git a/Simplenote/SPTextView.swift b/Simplenote/SPTextView.swift index 6680767c3..421b23118 100644 --- a/Simplenote/SPTextView.swift +++ b/Simplenote/SPTextView.swift @@ -160,7 +160,6 @@ extension SPTextView { return 0.0 } - ensureLayout() let textContainerHeight = layoutManager.usedRect(for: textContainer).size.height let textContainerMinHeight = scrollView.frame.size.height - scrollView.scrollerInsets.top return max(textContainerHeight, textContainerMinHeight) From 19199e4fec776bf2bd5863131a4ac5f778aef5b9 Mon Sep 17 00:00:00 2001 From: Dan Roundhill Date: Wed, 8 Jan 2025 15:22:09 -0700 Subject: [PATCH 2/2] Some thread safety checks. --- Simplenote/NSTextView+Simplenote.swift | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/Simplenote/NSTextView+Simplenote.swift b/Simplenote/NSTextView+Simplenote.swift index b40290231..85be5ee92 100644 --- a/Simplenote/NSTextView+Simplenote.swift +++ b/Simplenote/NSTextView+Simplenote.swift @@ -151,6 +151,9 @@ extension NSTextView { return } + // Capture the current content hash to detect changes + let contentHash = attributedString.string.hash + DispatchQueue.global(qos: .userInitiated).async { [weak self] in // Create a data detector for links let types = NSTextCheckingResult.CheckingType.link.rawValue @@ -163,12 +166,22 @@ extension NSTextView { let matches = detector.matches(in: attributedString.string, options: [], range: fullRange) // Apply the changes on the main queue - DispatchQueue.main.async { + DispatchQueue.main.async { [weak self] in guard let self = self, let textStorage = self.textStorage else { return } + // Verify the content hasn't changed while we were processing + guard contentHash == textStorage.string.hash else { + return + } + + // Verify ranges are still valid + guard fullRange.upperBound <= textStorage.length else { + return + } + // Disable the Delegate temporarily let theDelegate = self.delegate self.delegate = nil @@ -184,11 +197,16 @@ extension NSTextView { // Add link attributes while preserving existing ones for match in matches where match.resultType == .link { + // Verify each match range is still valid + guard match.range.upperBound <= textStorage.length else { + continue + } + if let url = match.url { textStorage.addAttribute(.link, value: url, range: match.range) } else { // If no URL, create one from the matched text - let text = attributedString.string as NSString + let text = textStorage.string as NSString let linkText = text.substring(with: match.range) if let url = URL(string: linkText) { textStorage.addAttribute(.link, value: url, range: match.range)