Skip to content

Commit

Permalink
Fixed a bug in TextStorage that would prevent textkit update notifica…
Browse files Browse the repository at this point in the history
…tions for character changes
  • Loading branch information
rajdeep committed Nov 16, 2024
1 parent a134e7d commit e5102ea
Show file tree
Hide file tree
Showing 17 changed files with 12 additions and 70 deletions.
2 changes: 1 addition & 1 deletion Proton/Sources/ObjC/PRTextStorage.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
9 changes: 9 additions & 0 deletions Proton/Sources/Swift/Core/RichTextView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 2 additions & 33 deletions Proton/Sources/Swift/Editor/EditorView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
}
Expand Down
15 changes: 0 additions & 15 deletions Proton/Sources/Swift/TextProcessors/TextProcessor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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) }
Expand Down
6 changes: 0 additions & 6 deletions Proton/Tests/Editor/EditorSnapshotTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions Proton/Tests/Editor/EditorViewContextTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ class EditorViewContextTests: XCTestCase {

func testCarriesOverCustomTypingAttributes() {
let editor = EditorView()
editor.forceApplyAttributedText = true
let context = EditorViewContext.shared
context.richTextViewContext.textViewDidBeginEditing(editor.richTextView)

Expand All @@ -80,7 +79,6 @@ class EditorViewContextTests: XCTestCase {

func testLockedAttributes() {
let editor = EditorView()
editor.forceApplyAttributedText = true
let context = EditorViewContext.shared
context.richTextViewContext.textViewDidBeginEditing(editor.richTextView)

Expand Down
1 change: 0 additions & 1 deletion Proton/Tests/Editor/EditorViewMenuTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ class TestEditorView: EditorView {

init() {
super.init()
forceApplyAttributedText = true
}

required init?(coder aDecoder: NSCoder) {
Expand Down
5 changes: 0 additions & 5 deletions Proton/Tests/Editor/EditorViewTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion Proton/Tests/Editor/EditorViewportSnapshotTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 0 additions & 1 deletion Proton/Tests/Grid/GridViewAttachmentSnapshotTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion Proton/Tests/Helpers/EditorTestViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class EditorTestViewController: SnapshotTestViewController {
private func setup() {
editor.translatesAutoresizingMaskIntoConstraints = false
editor.addBorder()
editor.forceApplyAttributedText = true

view.addSubview(editor)
NSLayoutConstraint.activate([
Expand Down
1 change: 0 additions & 1 deletion Proton/Tests/Table/TableViewAttachmentSnapshotTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 0 additions & 3 deletions Proton/Tests/TextProcessors/TextProcessorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit e5102ea

Please sign in to comment.