Fixed a bug in TextStorage that would prevent textkit update notifications for character changes

Author: rajdeep

Proton/Sources/ObjC/PRTextStorage.m

@@ -168,7 +168,7 @@ - (void)replaceCharactersInRange:(NSRange)range withString:(NSString *)str {
     NSInteger delta = str.length - range.length;
     [_storage replaceCharactersInRange:range withString:str];
     [_storage fixAttributesInRange:NSMakeRange(0, _storage.length)];
-    [self edited:NSTextStorageEditedCharacters & NSTextStorageEditedAttributes range:range changeInLength:delta];
+    [self edited:NSTextStorageEditedCharacters | NSTextStorageEditedAttributes range:range changeInLength:delta];
 
     [self endEditing];
 }

Proton/Sources/Swift/Core/RichTextView.swift

@@ -205,6 +205,15 @@ class RichTextView: AutogrowingTextView {
         draw(CGRect(origin: .zero, size: contentSize))
     }
 
+    override func becomeFirstResponder() -> Bool {
+        let didBecomeFirstResponder = super.becomeFirstResponder()
+        if didBecomeFirstResponder {
+            context?.selectedTextView = self
+            context?.activeTextView = self
+        }
+        return didBecomeFirstResponder
+    }
+
     func updateSelectedRangeIgnoringCallback(_ selectedRange: NSRange) {
         ignoreSelectedRangeChangeCallback = true
         self.selectedRange = selectedRange

Proton/Sources/Swift/Editor/EditorView.swift

@@ -150,12 +150,6 @@ open class EditorView: UIView {
         }
     }
 
-
-    // Holds `attributedText` until Editor move to a window
-    // Setting attributed text without Editor being fully ready
-    // causes issues with cached bounds that shows up when rotating the device.
-    private var pendingAttributedText: NSAttributedString?
-
     var editorContextDelegate: EditorViewDelegate? {
         get { editorViewContext.delegate }
     }
@@ -172,12 +166,6 @@ open class EditorView: UIView {
     /// Context for the current Editor
     public let editorViewContext: EditorViewContext
 
-    /// Returns if `attributedText` change is pending. `AttributedText` may not have been applied if the `EditorView` is not already on
-    /// `window` and `forceApplyAttributedText` is not set to `true`.
-    public var isAttributedTextPending: Bool {
-        pendingAttributedText != nil
-    }
-
     /// Enables asynchronous rendering of attachments.
     /// - Note:
     /// Since attachments must me rendered on main thread, the rendering only continues when there is no user interaction. By default, rendering starts
@@ -419,7 +407,6 @@ open class EditorView: UIView {
     /// An attachment is only counted as a single character. Content length does not include
     /// length of content within the Attachment that is hosting another `EditorView`.
     public var contentLength: Int {
-        guard pendingAttributedText == nil else { return attributedText.length }
         return richTextView.contentLength
     }
 
@@ -539,35 +526,20 @@ open class EditorView: UIView {
         }
     }
 
-    /// Forces setting attributed text in `EditorView` even if it is not
-    /// yet in view hierarchy.
-    /// - Note: This may result in misplaced `Attachment`s and is recommended to be set to `true` only in unit tests.
-    public var forceApplyAttributedText = false
-
     /// Text to be set in the `EditorView`
-    /// - Important: `attributedText` is not set for rendering in `EditorView` if the `EditorView` is not already in a `Window`. Value of `true`
-    /// for `isAttributedTextPending` confirms that the text has not yet been rendered even though it is set in the `EditorView`.
-    /// Notification of text being set can be observed by subscribing to `didSetAttributedText` in `EditorViewDelegate`.
-    /// Alternatively, `forceApplyAttributedText` may be set to `true` to always apply `attributedText` irrespective of `EditorView` being
-    /// in a `Window` or not.
     public var attributedText: NSAttributedString {
         get {
-            pendingAttributedText ?? richTextView.attributedText
+            richTextView.attributedText
         }
         set {
-            if forceApplyAttributedText == false && window == nil {
-                pendingAttributedText = newValue
-                return
-            }
             isSettingAttributedText = true
             attachmentRenderingScheduler.cancel()
             renderedViewport = nil
             // Clear text before setting new value to avoid issues with formatting/layout when
             // editor is hosted in a scrollable container and content is set multiple times.
             richTextView.attributedText = NSAttributedString()
 
-            let isDeferred = pendingAttributedText != nil
-            pendingAttributedText = nil
+            let isDeferred = false
 
             AggregateEditorViewDelegate.editor(self, willSetAttributedText: newValue, isDeferred: isDeferred)
 
@@ -872,9 +844,6 @@ open class EditorView: UIView {
     /// - IMPORTANT: Overriding implementations must call `super.didMoveToWindow()`
     open override func didMoveToWindow() {
         super.didMoveToWindow()
-        if let pendingAttributedText {
-            attributedText = pendingAttributedText
-        }
         let isReady = window != nil
         AggregateEditorViewDelegate.editor(self, isReady: isReady)
     }

Proton/Sources/Swift/TextProcessors/TextProcessor.swift

@@ -64,8 +64,6 @@ class TextProcessor: NSObject, NSTextStorageDelegate {
         var processed = false
         let changedText = textStorage.substring(from: editedRange)
 
-        let editedMask = getEditedMask(delta: delta)
-
         let executableProcessors = filteringExecutableOn(editor: editor)
 
         executableProcessors.forEach {
@@ -97,7 +95,6 @@ class TextProcessor: NSObject, NSTextStorageDelegate {
 
     func textStorage(_ textStorage: NSTextStorage, didProcessEditing editedMask: NSTextStorage.EditActions, range editedRange: NSRange, changeInLength delta: Int) {
         guard let editor = editor else { return }
-        let editedMask = getEditedMask(delta: delta)
         let executableProcessors = filteringExecutableOn(editor: editor)
 
         executableProcessors.forEach {
@@ -113,18 +110,6 @@ class TextProcessor: NSObject, NSTextStorageDelegate {
         }
     }
 
-    // The editedMask is computed here as fixing the actual bug in PRTextStorage.replaceCharacter ([self edited:])
-    // causing incorrect editedMask coming-in in this delegate causes TableViewAttachmentSnapshotTests.testRendersTableViewAttachmentInViewportRotation
-    // to hang, possibly due to persistent layout invalidations. This can be fixed if cell has foreApplyAttributedText on
-    // which ensures TextStorage to always be consistent state. However, given that there is some unknown, the proper fix
-    // in PRTextStorage will be added at a later time. It may include dropping need for forceApplyAttributedText.
-    private func getEditedMask(delta: Int) -> NSTextStorage.EditActions {
-        guard delta != 0 else {
-            return .editedAttributes
-        }
-        return [.editedCharacters, .editedAttributes]
-    }
-
     private func notifyInterruption(by processor: TextProcessing, editor: EditorView, at range: NSRange) {
         let processors = activeProcessors.filter { $0.name != processor.name }
         processors.forEach { $0.processInterrupted(editor: editor, at: range) }

Proton/Tests/Editor/EditorSnapshotTests.swift

@@ -99,7 +99,6 @@ class EditorSnapshotTests: SnapshotTestCase {
         editor.font = UIFont.systemFont(ofSize: 12)
 
         var panel = PanelView()
-        panel.editor.forceApplyAttributedText = true
         panel.backgroundColor = .cyan
         panel.layer.borderWidth = 1.0
         panel.layer.cornerRadius = 4.0
@@ -266,7 +265,6 @@ class EditorSnapshotTests: SnapshotTestCase {
 
         for i in 1...10 {
             var panel = PanelView()
-            panel.editor.forceApplyAttributedText = true
             panel.backgroundColor = .cyan
             panel.layer.borderWidth = 1.0
             panel.layer.cornerRadius = 4.0
@@ -425,7 +423,6 @@ class EditorSnapshotTests: SnapshotTestCase {
         editor.attributedText = NSAttributedString(string: "One\nTwo\nThree")
 
         var panel = PanelView()
-        panel.editor.forceApplyAttributedText = true
         panel.backgroundColor = .cyan
         panel.layer.borderWidth = 1.0
         panel.layer.cornerRadius = 4.0
@@ -466,7 +463,6 @@ class EditorSnapshotTests: SnapshotTestCase {
         let attachment = Attachment(panel, size: .fullWidth)
         panel.boundsObserver = attachment
         panel.editor.font = editor.font
-        panel.editor.forceApplyAttributedText = true
 
         panel.attributedText = NSAttributedString(string: "In \nfull-width \nattachment")
 
@@ -1421,7 +1417,6 @@ class EditorSnapshotTests: SnapshotTestCase {
         editor.font = UIFont.systemFont(ofSize: 12)
 
         var panel = PanelView()
-        panel.editor.forceApplyAttributedText = true
         panel.backgroundColor = .cyan
         panel.layer.borderWidth = 1.0
         panel.layer.cornerRadius = 4.0
@@ -1455,7 +1450,6 @@ class EditorSnapshotTests: SnapshotTestCase {
         editor.font = UIFont.systemFont(ofSize: 12)
 
         var panel = PanelView()
-        panel.editor.forceApplyAttributedText = true
         panel.backgroundColor = .cyan
         panel.layer.borderWidth = 1.0
         panel.layer.cornerRadius = 4.0

Proton/Tests/Editor/EditorViewContextTests.swift

@@ -63,7 +63,6 @@ class EditorViewContextTests: XCTestCase {
 
     func testCarriesOverCustomTypingAttributes() {
         let editor = EditorView()
-        editor.forceApplyAttributedText = true
         let context = EditorViewContext.shared
         context.richTextViewContext.textViewDidBeginEditing(editor.richTextView)
 
@@ -80,7 +79,6 @@ class EditorViewContextTests: XCTestCase {
 
     func testLockedAttributes() {
         let editor = EditorView()
-        editor.forceApplyAttributedText = true
         let context = EditorViewContext.shared
         context.richTextViewContext.textViewDidBeginEditing(editor.richTextView)
 

Proton/Tests/Editor/EditorViewMenuTests.swift

@@ -128,7 +128,6 @@ class TestEditorView: EditorView {
 
     init() {
         super.init()
-        forceApplyAttributedText = true
     }
 
     required init?(coder aDecoder: NSCoder) {

Proton/Tests/Editor/EditorViewTests.swift

@@ -224,7 +224,6 @@ class EditorViewTests: XCTestCase {
 
         let delegate = MockEditorViewDelegate()
         let editor = EditorView()
-        editor.forceApplyAttributedText = true
         let attachment = Attachment(PanelView(), size: .fullWidth)
         let attrString = NSMutableAttributedString(string: "This is a test string")
         attrString.append(attachment.string)
@@ -386,7 +385,6 @@ class EditorViewTests: XCTestCase {
 
     func testReturnsNilForInvalidNextLine() throws {
         let editor = EditorView()
-        editor.forceApplyAttributedText = true
         let attrString = NSMutableAttributedString(string: "This is a test string")
         editor.attributedText = attrString
 
@@ -397,7 +395,6 @@ class EditorViewTests: XCTestCase {
 
     func testReturnsNilForInvalidPreviousLine() throws {
         let editor = EditorView()
-        editor.forceApplyAttributedText = true
         let attrString = NSMutableAttributedString(string: "This is a test string")
         editor.attributedText = attrString
 
@@ -408,7 +405,6 @@ class EditorViewTests: XCTestCase {
 
     func testResetsAttributesWhenCleared() {
         let editor = EditorView()
-        editor.forceApplyAttributedText = true
         editor.textColor = UIColor.red
         let attrString = NSMutableAttributedString(string: "This is a test string", attributes: [.foregroundColor: UIColor.blue])
         editor.attributedText = attrString
@@ -924,7 +920,6 @@ class EditorViewTests: XCTestCase {
 
 func makePanelAttachment() -> Attachment {
     let panel = PanelView()
-    panel.editor.forceApplyAttributedText = true
     panel.backgroundColor = .cyan
     panel.layer.borderWidth = 1.0
     panel.layer.cornerRadius = 4.0

Proton/Tests/Editor/EditorViewportSnapshotTests.swift

@@ -168,7 +168,6 @@ class EditorViewportSnapshotTests: SnapshotTestCase {
 
         for i in 0..<count {
             var panel = PanelView()
-            panel.editor.forceApplyAttributedText = true
             panel.backgroundColor = .cyan
             panel.layer.borderWidth = 1.0
             panel.layer.cornerRadius = 4.0

Proton/Tests/Editor/__Snapshots__/EditorListsSnapshotTests/testFullWidthAttachmentInLists.1.png

@@ -158,7 +158,6 @@ class GridViewAttachmentSnapshotTests: SnapshotTestCase {
             ])
         let gridAttachment = GridViewAttachment(config: config)
         var panel = PanelView()
-        panel.editor.forceApplyAttributedText = true
         panel.backgroundColor = .cyan
         panel.layer.borderWidth = 1.0
         panel.layer.cornerRadius = 4.0

Proton/Tests/Editor/__Snapshots__/EditorListsSnapshotTests/testFullWidthAttachmentInLists.2.png

@@ -41,7 +41,6 @@ class EditorTestViewController: SnapshotTestViewController {
     private func setup() {
         editor.translatesAutoresizingMaskIntoConstraints = false
         editor.addBorder()
-        editor.forceApplyAttributedText = true
 
         view.addSubview(editor)
         NSLayoutConstraint.activate([

Proton/Tests/EditorCommands/__Snapshots__/EditorCommandSnapshotTests/testExecutesCommandOnNestedEditors.1.png

@@ -191,7 +191,6 @@ class TableViewAttachmentSnapshotTests: SnapshotTestCase {
             ])
         let tableAttachment = makeTableViewAttachment(config: config)
         var panel = PanelView()
-        panel.editor.forceApplyAttributedText = true
         panel.backgroundColor = .cyan
         panel.layer.borderWidth = 1.0
         panel.layer.cornerRadius = 4.0

Proton/Tests/Grid/GridViewAttachmentSnapshotTests.swift

@@ -220,7 +220,6 @@ class TextProcessorTests: XCTestCase {
         processorExpectation.expectedFulfillmentCount = 3
 
         let editor = EditorView()
-        editor.forceApplyAttributedText = true
         editor.attributedText = NSAttributedString(string: "Test")
         let testAttribute = NSAttributedString.Key("testAttr")
 
@@ -252,7 +251,6 @@ class TextProcessorTests: XCTestCase {
     func testGetsNotifiedOfSelectedRangeChanges() {
         let testExpectation = functionExpectation()
         let editor = EditorView()
-        editor.forceApplyAttributedText = true
         let name = "TextProcessorTest"
         let mockProcessor = MockTextProcessor(name: name)
         let originalRange = NSRange(location: 2, length: 1)
@@ -529,7 +527,6 @@ class TextProcessorTests: XCTestCase {
 
     private func assertProcessorInvocationOnSetAttributedText(_ expectation: XCTestExpectation, isRunOnSettingText: Bool, file: StaticString = #file, line: UInt = #line, assertion: (MockTextProcessor) -> Void) throws {
         let editor = EditorView()
-        editor.forceApplyAttributedText = true
 
         let name = "TextProcessorTest"
         let mockProcessor = MockTextProcessor(name: name)